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

js2-PRIVATE_NAME is breaking js2-skip-preprocessor-directives #585

Open
sandinmyjoints opened this issue Apr 1, 2022 · 7 comments
Open

Comments

@sandinmyjoints
Copy link

sandinmyjoints commented Apr 1, 2022

Repro:

  1. (setq js2-skip-preprocessor-directives t)
  2. create a js file with this content:
#!/usr/bin/env node

Check js2-parsed-errors. It should be nil, but it will be:

(((#1="msg.no.semi.stmt" nil)
  16 4)
 ((#1# nil)
  8 3)
 (("msg.invalid.re.flag" nil)
  8 1)
 ((#1# nil)
  2 1)
 (("msg.syntax" nil)
  1 1))

js2-mode thinks the token type of the first character (#) is 177, which is js2-PRIVATE_NAME, introduced in fc82b04 to address #537

@dgutov
Copy link
Collaborator

dgutov commented Apr 2, 2022

Hi! These features conflict quite a bit, though.

How do we disambiguate? Do we disable private names altogether when js2-skip-preprocessor-directives is non-nil?

Or do we keep the list of allowed PP directives? And/or perhaps exclude #!/ directives some other way.

@sandinmyjoints
Copy link
Author

Hello! I don't know if there's an official spec for shebangs, but https://en.wikipedia.org/wiki/Shebang_(Unix) says that they only appear at the very beginning of a script, and that's where I've always seen them. I believe that when # is the first character of a file, it could not be a valid use of a private name in JS (though I haven't read its spec to confirm). So it seems like their usages may not actually overlap.

Would it be possible for js2-mode to do a one-off check of the very first character in a file, and if it's #, then skip to the next line and start parsing?

@dgutov
Copy link
Collaborator

dgutov commented Apr 2, 2022

Yeah, that would be easy enough. Just skip it before starting to parse. Added that on master.

I was wondering also whether somebody has ideas on what to do with actual preprocessor directives, like in https://github.com/mozilla/gecko-dev/blob/master/browser/app/profile/firefox.js#L16-L20

But maybe Firefox devs don't use this package anymore.

dgutov added a commit that referenced this issue Apr 2, 2022
@UwUnyaa
Copy link

UwUnyaa commented Apr 3, 2022

What about considering the context of the # character? Private fields don't make sense outside of class declarations.

@dgutov
Copy link
Collaborator

dgutov commented Apr 3, 2022

That's an option, but preprocessor macros could be used inside class declarations, I suppose.

@sandinmyjoints
Copy link
Author

Thanks @dgutov. My issue is solved -- I only use shebangs, not preprocessor directives. I can leave this open, though, since it seems the issue of fixing preprocessor directives is still open.

@dgutov
Copy link
Collaborator

dgutov commented Apr 10, 2022

That's the idea.

And thanks for checking.

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

No branches or pull requests

3 participants