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

Add support for hidden_file_extensions key. #419

Merged

Conversation

varungandhi-src
Copy link
Contributor

@varungandhi-src varungandhi-src commented Jan 16, 2022

This was introduced in Sublime Text 4.

Addresses one part of #323.

As I understand it, it doesn't really matter if a file extension is "hidden" or not from the POV of syntax highlighting.

@varungandhi-src
Copy link
Contributor Author

Is anyone actively working on fixing 323/adding support for the other new features? I didn't see any active PRs related to it, and no one had commented on the issue, but I figured I'd ask first.

@keith-hall
Copy link
Collaborator

Is anyone actively working on fixing 323/adding support for the other new features?

Not that I know of, no. Iirc I wrote a comment asking for advice on how the API might look like, but had no responses. I did have a quick look at implementing support for popping multiple contexts at once but it wasn't trivial with the current design.

Copy link
Owner

@trishume trishume left a comment

Choose a reason for hiding this comment

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

I don't think anyone else is. However note that the real trickiness will be the backtracking support, which I believe will require a complete API overhaul, since currently you can parse in a streaming way since no line can affect a previous line, whereas with backtracking that's no longer true.

I think it might be reasonable to support everything except backtracking though, since there'll be lots of syntaxes which might use some new features but not backtracking.

@trishume
Copy link
Owner

Oh, see the minimum supported rust version CI error, you just need to borrow the array you're iterating.

@varungandhi-src varungandhi-src force-pushed the vg/upstream/hidden-file-extensions branch from 7cce8b4 to 725cdf4 Compare January 17, 2022 04:26
@varungandhi-src
Copy link
Contributor Author

CI error should be fixed with the latest push.

As for backtracking, thanks for bringing that up. I'd only seen issue 323 and not issue 271, so I hadn't realized that was another potential pain point before bumping to ST4. It looks like a bunch of grammars are already using the backtracking-related features:

Batch File/Batch File.sublime-syntax
C#/C#.sublime-syntax
Git Formats/Git Commit.sublime-syntax
Java/Java.sublime-syntax
JavaScript/JSX.sublime-syntax
JavaScript/JavaScript.sublime-syntax
JavaScript/TSX.sublime-syntax
JavaScript/TypeScript.sublime-syntax
Lua/Lua.sublime-syntax
Markdown/Markdown.sublime-syntax
PHP/PHP Source.sublime-syntax
Python/Python.sublime-syntax
SQL/SQL.sublime-syntax

Apart from backtracking, it sounds like adding support for the 4075-specific features, such as syntax inheritance, is not as complicated and shouldn't affect the public API -- is that a fair assessment?

@jrappen
Copy link
Contributor

jrappen commented Jan 17, 2022

... grammars which are using branch: ...

also add these pending re-writes from PRs:

  • Haskell
  • JSON

@varungandhi-src
Copy link
Contributor Author

Could someone please kick-off the PR testing again? The clippy issue should be fixed, and I'm seeing a "First-time contributors need a maintainer to approve running workflows" message.

@trishume
Copy link
Owner

Huh I tried re-running it and the clippy issue happened again. Did you already look into it? Looks like the problem is that the time crate updated past our MSRV, which of course isn't a problem with this PR. Perhaps we need to either bump our MSRV or constrain an older version of the time crate.

@Enselic
Copy link
Collaborator

Enselic commented Jan 23, 2022

I created a PR to bump MSRV along with a motivation of why I believe that is preferred over putting an upper limit on time: #422

@varungandhi-src
Copy link
Contributor Author

Thanks @Enselic, we can retest and land this PR after your PR lands.

@Enselic
Copy link
Collaborator

Enselic commented Mar 14, 2022

@varungandhi-src Could you merge origin/master into this branch so CI becomes green please?

This was introduced in Sublime Text 4.
@varungandhi-src varungandhi-src force-pushed the vg/upstream/hidden-file-extensions branch from 725cdf4 to e54eef3 Compare March 14, 2022 17:00
@varungandhi-src
Copy link
Contributor Author

varungandhi-src commented Mar 14, 2022

@Enselic rebased and force-pushed; sorry I hadn't realized that PR testing was running pre-merge instead of after locally merging into master.

@Enselic
Copy link
Collaborator

Enselic commented Mar 14, 2022

Since this has been Approved by both Tristan and Keith I will merge this now that CI is green. I took a quick look and the code looked good to me too.

I will add an entry for this PR to CHANGELOG at a later point as part of #409

@Enselic Enselic merged commit 6b211f9 into trishume:master Mar 14, 2022
@varungandhi-src varungandhi-src deleted the vg/upstream/hidden-file-extensions branch March 14, 2022 19:40
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.

None yet

5 participants