10000 feat(v): prefer to generate from grammar by ttytm · Pull Request #6300 · nvim-treesitter/nvim-treesitter · GitHub
[go: up one dir, main page]

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(v): prefer to generate from grammar #6300

Closed
wants to merge 1 commit into from
Closed

feat(v): prefer to generate from grammar #6300

wants to merge 1 commit into from

Conversation

ttytm
Copy link
Contributor
@ttytm ttytm commented Mar 15, 2024

No description provided.

@clason
Copy link
Contributor
clason commented Mar 15, 2024

Why? You should prefer to compile C, as this does not require treeitter CLI

@ttytm
Copy link
Contributor Author
ttytm commented Mar 15, 2024

The goal is more convenience. Not keeping the generated files not in the repository but having it automatically generated at the latest development state on client side has some advantages.

From what I experienced at the v-analyzer repos:
Updating the generated files in the repo upon changes to the grammar is additional effort and adds error potential.

Also in terms of the growth of the repository size. E.g. simple changes that are part the commit history but produce a diff of ~200k lines are v-analyzer/v-analyzer#65, v-analyzer/v-analyzer#74. During early development this has more impact and the project is fairly young.

So I thought it's a good way and an approach e.g. Swift is choosing.

@ttytm
Copy link
Contributor Author
ttytm commented Mar 15, 2024

Let's wait until another maintainer of v-analyzer is confirming that generating the parser is a desired approach.

@clason
Copy link
Contributor
clason commented Mar 15, 2024

As long as the parser.c is committed, you should not set this flag. It's only for repos that do not contain the generated parser.

@clason clason closed this Mar 15, 2024
@ttytm
Copy link
Contributor Author
ttytm commented Mar 15, 2024

You checked the related PR at v-analyzer? It would remove the parser.c

@ttytm
Copy link
Contributor Author
ttytm commented Mar 15, 2024

Also, there are tree-sitter projects that have parser.c in their repo but requires_generate_from_grammar set to true.

@clason
Copy link
Contributor
clason commented Mar 15, 2024

Then you should make a PR to remove the flag.

@clason
Copy link
Contributor
clason commented Mar 15, 2024

You checked the related PR at v-analyzer? It would remove the parser.c

Once this is merged, we will need to adapt. But not a second sooner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0