mooz / js2-mode

Improved JavaScript editing mode for GNU Emacs
GNU General Public License v3.0
1.33k stars 186 forks source link

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

Open sandinmyjoints opened 2 years ago

sandinmyjoints commented 2 years ago

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 https://github.com/mooz/js2-mode/commit/fc82b042fd12d8bb4d66750d4f392d40431d8a87 to address https://github.com/mooz/js2-mode/issues/537

dgutov commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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.

UwUnyaa commented 2 years ago

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

dgutov commented 2 years ago

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

sandinmyjoints commented 2 years ago

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 commented 2 years ago

That's the idea.

And thanks for checking.