ralsina / tartrazine

A Crystal reimplementation of the Pygments/Chroma syntax highlighters
MIT License
9 stars 1 forks source link

More control over baked lexers #8

Closed devnote-dev closed 6 days ago

devnote-dev commented 3 weeks ago

Currently, all 247 lexer files (+ 2 license files) are baked into every binary when using Tartrazine; needless to say this is far from ideal. I propose having a public API to allow selecting lexers to include at compile time. This would mean that lexers selected during runtime would be limited to the ones included at compile time, but I think that would be better and give users more control over what they actually want to use.

ralsina commented 3 weeks ago

@devnote-dev you can just delete the files you don´t want after shard install and re-run the script that generates constants, since it does no integrity check.

ralsina commented 3 weeks ago

The difference between including everything and including the bare minimum is about 3.3MB, which is significant. I can mitigate it by gzipping the files, or we can figure out a way to limit what gets included

devnote-dev commented 3 weeks ago

The issue isn't so much binary size, but I'd argue that there's no point in having so many unused lexers/themes if you only need one. My main issue is with compile times: when testing Tartrazine with a CLI project I'm working on, the compile times went from the usual 40-ish seconds to ~2 minutes (and these are debug builds).

ralsina commented 3 weeks ago

Yikes, that sucks!

I may be a bit spoiled because my machine builds tartrazine in 3 seconds :-D

Sure, let's figure it out. Idea: env vars set before build

TT_INCLUDE_LEXERS="list,of,lexers,to,include" TT_INCLUDE_THEMES="list,of,themes" TT_NO_BASE16_THEMES="true" # Doesn't include the base16 themes and just keeps the others

devnote-dev commented 3 weeks ago

That's one way to do it, but would it perform compile time validation? That would guard against unknown lexers/themes being passed as arguments.

ralsina commented 3 weeks ago

It should keep the current behaviour, when you pass an unknown lexer it will throw an exception.

I will take a shot at this tonight at home.

ralsina commented 3 weeks ago

Check #9 to see if it works for you. That is for lexers, need to work in another project for themes.

ralsina commented 3 weeks ago

Merged #9 which fixes this for lexers, next release of github.com/ralsina/sixteen will fix it for themes

ralsina commented 1 week ago

Release v0.4.0 of sixteen adds support to choose themes to be included.

devnote-dev commented 1 week ago

Apologies for the radio silence, I've been having a weird time with GitHub notifications recently...

Following your instructions I've installed Tartrazine and tried to limit it to the Crystal lexer as that's all I need, however, the following issues occur:

  1. Sixteen still expects the SIXTEEN_THEMES variable to be set when compiling with -Dnothemes
  2. When setting SIXTEEN_THEMES="" this error appears:

    Showing last frame. Use --error-trace for full trace.
    
    In lib\sixteen\src\sixteen.cr:15:45
    
     15 | {% for theme in env("SIXTEEN_THEMES").split "," %}
                                                ^----
    Error: undefined macro method 'NilLiteral#split'

Additionally, these changes only work for Sixteen's theme files; there's no way to specify Tartrazine's XML styles.

ralsina commented 1 week ago

I'll check that error. But yes, there is no way yet to limit those yet.

On Sun, Oct 13, 2024, 12:55 PM Devonte @.***> wrote:

Apologies for the radio silence, I've been having a weird time with GitHub notifications recently...

Following your instructions I've installed Tartrazine and tried to limit it to the Crystal lexer as that's all I need, however, the following issues occur:

1.

Sixteen still expects the SIXTEEN_THEMES variable to be set when compiling with -Dnothemes 2.

When setting SIXTEEN_THEMES="" this error appears:

Showing last frame. Use --error-trace for full trace.

In lib\sixteen\src\sixteen.cr:15:45

15 | {% for theme in env("SIXTEEN_THEMES").split "," %}
                                           ^----

Error: undefined macro method 'NilLiteral#split'

Additionally, these changes only work for Sixteen's theme files; there's no way to specify Tartrazine's XML styles.

— Reply to this email directly, view it on GitHub https://github.com/ralsina/tartrazine/issues/8#issuecomment-2409030077, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAMKZXFECZCCUFAXIHEWTZ3KJWZAVCNFSM6AAAAABOWQNXY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBZGAZTAMBXG4 . You are receiving this because you modified the open/close state.Message ID: @.***>

ralsina commented 6 days ago
ralsina commented 6 days ago

Fixed in sixteen 0.4.2 and tartrazine 0.11.0, see the README for details

devnote-dev commented 6 days ago

Unfortunately this still does not work...

I tried compiling specifying the crystal lexer (at which I then discovered that it's built into the shard), but it led me to discover this issue when specifying any lexers:

In lib\tartrazine\src\lexer.cr:12:40

 12 | bake_file {{ lexer }}+".xml", {{ read_file "lexers/" + lexer + ".xml" }}
                                       ^--------
Error: Error opening file with mode 'r': 'lexers/apl.xml': The system cannot find the path specified.

I believe this should be using the directory path of the file. I was able to compile my app without this but then came across this issue when running app:

Unhandled exception: Theme default-dark not found (Exception)
  from lib\tartrazine\src\styles.cr:39 in 'theme'
  from lib\tartrazine\src\formatter.cr:12 in 'new:theme:line_numbers'
  from lib\tartrazine\src\formatters\ansi.cr:7 in 'to_ansi:text:language:theme'
  from src\renderer.cr:33 in 'code_block'
  from lib\markd\src\markd\renderer.cr:70 in 'render'
  from src\renderer.cr:112 in 'render'
...

I have no idea what's causing this, I'm using the following code:

Tartrazine.to_ansi(text: node.text, language: "crystal", theme: "github-dark").each_line do |line|
  io << "  " << line << '\n'
end
ralsina commented 6 days ago

This should fix it:

 macro bake_selected_lexers
   {% for lexer in env("TT_LEXERS").split "," %}

I'll do a fixed release later.

On Mon, Oct 14, 2024 at 3:30 PM Devonte @.***> wrote:

Unfortunately this still does not work...

I tried compiling specifying the crystal lexer (at which I then discovered that it's built into the shard), but it led me to discover this issue when specifying any lexers:

In lib\tartrazine\src\lexer.cr:12:40

12 | bake_file {{ lexer }}+".xml", {{ read_file "lexers/" + lexer + ".xml" }} ^-------- Error: Error opening file with mode 'r': 'lexers/apl.xml': The system cannot find the path specified.

I believe this should be using the directory path of the file. I was able to compile my app without this but then came across this issue when running app:

Unhandled exception: Theme default-dark not found (Exception) from lib\tartrazine\src\styles.cr:39 in 'theme' from lib\tartrazine\src\formatter.cr:12 in 'new:theme:line_numbers' from lib\tartrazine\src\formatters\ansi.cr:7 in 'to_ansi:text:language:theme' from src\renderer.cr:33 in 'code_block' from lib\markd\src\markd\renderer.cr:70 in 'render' from src\renderer.cr:112 in 'render' ...

I have no idea what's causing this, I'm using the following code:

Tartrazine.to_ansi(text: node.text, language: "crystal", theme: "github-dark").each_line do |line| io << " " << line << '\n'end

— Reply to this email directly, view it on GitHub https://github.com/ralsina/tartrazine/issues/8#issuecomment-2411965498, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAMK3BLEDPNX2DS5OQBKDZ3QET5AVCNFSM6AAAAABOWQNXY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMJRHE3DKNBZHA . You are receiving this because you modified the open/close state.Message ID: @.***>

ralsina commented 6 days ago

Fixed in 0.11.1