Skip to content

Conversation

@mjambon
Copy link
Contributor

@mjambon mjambon commented Sep 14, 2021

Without this, the following extra files get generated when we run npm run build:

	tsx/Cargo.toml
	tsx/binding.gyp
	tsx/bindings/
	typescript/Cargo.toml
	typescript/binding.gyp
	typescript/bindings/

Files with such names already exist under the project's root e.g. https://github.com/tree-sitter/tree-sitter-typescript/blob/master/Cargo.toml.

I don't know what these files are for so I don't know if this PR does the right thing :-). Maybe the right behavior is instead to keep those under tsx/ and typescript/ and delete the ones under the project root?

Checklist:

  • All tests pass in CI.
  • There are sufficient tests for the new fix/feature.
  • Grammar rules have not been renamed unless absolutely necessary.
  • The conflicts section hasn't grown too much.
  • The parser size hasn't grown too much (check the value of STATE_COUNT in src/parser.c).

@mjambon
Copy link
Contributor Author

mjambon commented Sep 14, 2021

The issues of these new generated files was raised in #135 and #163

Copy link
Contributor

@dcreager dcreager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 For this particular repo, this is absolutely the right thing to do, because we already have these bindings files in the repo root. So the new files being generated in the tsx and typescript directories are redundant with the files that already exist.

(The Cargo.toml is needed to publish the Rust crate for this parser, binding.gyp is for the npm package, and the bindings/{node,rust} directories contain their implementations.)

@resolritter
Copy link
Contributor

FWIW when I tried to use --no-bindings locally on #135, I got the following:

> ../node_modules/.bin/tree-sitter generate --no-bindings
error: Found argument '--no-bindings' which wasn't expected, or isn't valid in this context

USAGE:
    tree-sitter generate [FLAGS] [OPTIONS] [grammar-path]

For more information try --help

> ../node_modules/.bin/tree-sitter --version
tree-sitter 0.19.2 (d037c4914d7845385dbf330f1a653777f81b7240)

@mjambon
Copy link
Contributor Author

mjambon commented Sep 15, 2021

@resolritter ah, that's good to know.

Here's the relevant section from tree-sitter's git log:

commit 24785cdb39ad2740ca33c111490984333787f5d3
Author: Max Brunsfeld <[email protected]>
Date:   Mon Mar 8 16:36:59 2021 -0800

    0.19.3

commit 9e50befcf8e1b6855337b7a877aa3099947f5678
Author: Max Brunsfeld <[email protected]>
Date:   Mon Mar 8 12:02:01 2021 -0800

    For node-types.json, process supertypes in a stable order

commit 8e894ff3f1898fcaa09ae125bbd5fde8467aea42
Author: Max Brunsfeld <[email protected]>
Date:   Mon Mar 8 12:01:45 2021 -0800

    Add --no-bindings flag to generate subcommand

So we need to set a constraint on tree-sitter >= 0.19.3.

@mjambon
Copy link
Contributor Author

mjambon commented Sep 15, 2021

I added a version constraint and also added the unwanted files to .gitignore as a precaution. Thanks @dcreager and @resolritter for your help.

@mjambon mjambon merged commit 7579fd0 into master Sep 15, 2021
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.

4 participants