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

Relax Downloads policy for new syntaxes #1262

Open
data-man opened this issue Oct 5, 2020 · 9 comments
Open

Relax Downloads policy for new syntaxes #1262

data-man opened this issue Oct 5, 2020 · 9 comments
Labels
feature-request New feature or request syntax-request

Comments

@data-man
Copy link

data-man commented Oct 5, 2020

As @sharkdp suggested.

If you like, you could open a new ticket to discuss the 10k downloads policy. I'm pretty sure it's not ideal, but I wanted to have something to limit the amount of syntaxes that we need to maintain. bat startup speed is also a big concern.

@data-man data-man added the question Further information is requested label Oct 5, 2020
@sharkdp sharkdp changed the title Downloads policy Relax Downloads policy for new syntaxes? Oct 5, 2020
@eth-p
Copy link
Collaborator

eth-p commented Oct 5, 2020

Startup speed is still a large concern, but I'm open to the idea of aggregating other download metrics together to get the 10k minimum.

For example, that would allow the following to meet the criteria:

3000 downloads from JetBrains plugins
7000 downloads from Sublime

It would have to be considered on a case by case basis, however. I wouldn't consider npm to be a valid metric, since something like a templating language could be a dependency of a larger and more popular framework, yet go unused by most projects installing the framework.

@sharkdp
Copy link
Owner

sharkdp commented Oct 5, 2020

We could also think about other (additional) policies like: the syntax can be added if GitHub also supports it.

Startup speed is still a large concern

We should probably measure the actual slowdown when adding more and more syntaxes. Shouldn't be too hard with some scripting and the available set of submodules.

@rsteube
Copy link
Contributor

rsteube commented Oct 6, 2020

@eth-p what's affecting the startup speed so much? Is it the single syntaxes.bin where they are all merged in?
And would it make sense to just split that in java.bin, python.bin... and load on demand?

@eth-p
Copy link
Collaborator

eth-p commented Oct 6, 2020

We did some digging in #951, and the majority of the startup time is spent deserializing the syntaxes.

And would it make sense to just split that in java.bin, python.bin... and load on demand?

It's always possible, but it would likely require a non-trivial amount of refactoring and possibly even a few upstream changes to do it, though. Most of the syntax detection (e.g. by file name, by first content line) is handled by Syntect, which relies on the information provided from the .sublime-syntax files which get serialized into syntaxes.bin.

Splitting the syntaxes.bin file up would likely require us to build a mapping of that information, manually resolve which asset set it belongs to, and then deserialize that set. To complicate it further, we would probably have to figure this all out before starting any of the printing... which means we need to add a syntax resolution stage to bat's setup (and that might involve opening all the files passed as arguments).

Basically, the main thing stopping someone from adding it is the significant amount of time, effort, upstream collaboration, and regression testing behind it. The silver lining is that if we did do it that way, we could also easily add syntax detection support for nix-shell (#684) though.

@rsteube
Copy link
Contributor

rsteube commented Oct 6, 2020

Probably not worth it then since the current solution is already quite good enough. I expect that the download limit / current set of embedded syntaxes won't be seen as a problem any more when the process of adding them gets a bit simpler.

@data-man
Copy link
Author

data-man commented Oct 6, 2020

We could also think about other (additional) policies like: the syntax can be added if GitHub also supports it.

👍

Another possible criterion: a language is popular on Repology.

@keith-hall
Copy link
Collaborator

Regarding nix-shell syntax detection support, @eth-p, I was thinking it could potentially be solved with a new syntax definition specifically for nix-shell files which would read the second line shebang and push into the relevant syntax, similar to how the Markdown syntax definition works with embedded code fence blocks.

To simplify it in terms of maintenance (with needing to have a rule for each "interpreter" we want highlighting support for, and potentially wanting to ensure any newly added syntax definitions are easily included) I think we could auto generate it with a script, which would look through each .sublime-syntax file, check if it has a first_line_match which matches a shebang line, and generate the relevant match rule to handle the nix-shell shebang line and set which language the rest of the file will be highlighted with.
(I suspect we would probably want to maintain or automate that mapping anyway, whichever solution we choose.)
I'm not yet sure how it would work with custom syntaxes the user has installed/configured - ideally it would cater for those too...

@eth-p
Copy link
Collaborator

eth-p commented Oct 6, 2020

@keith-hall Great idea! We might even be able to do that as part of the asset compilation, actually. I'm not as familiar with syntect as you are, but if there's a way to extract the first line mappings and syntax name, we can generate the syntax yaml on the fly as part of bat cache and load it into the final syntax set before serializing it.

@sharkdp sharkdp mentioned this issue Oct 7, 2020
5 tasks
@Enselic Enselic changed the title Relax Downloads policy for new syntaxes? Relax Downloads policy for new syntaxes Aug 4, 2021
@Enselic Enselic added syntax-request feature-request New feature or request and removed question Further information is requested labels Aug 4, 2021
@Enselic
Copy link
Collaborator

Enselic commented Aug 4, 2021

We can most likely do this to some extent once #951 is solved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request syntax-request
Projects
None yet
Development

No branches or pull requests

6 participants