ralsina / tartrazine

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

Non-trivial dependencies should be optional #10

Closed devnote-dev closed 6 days ago

devnote-dev commented 1 week ago

I was quite confused to see docopt, stumpy_core, stumpy_png and stumpy_utils being installed with Tartrazine, shards that I'm familiar with that wouldn't make sense for a library 🤔. I then discovered the first is for using the shard as an application and the latter for extensions. Realistically such shards shouldn't be required dependencies given they aren't trivial for Tartrazine to operate. Usually if an extension is to be used, it's explicitly required in the user's code at which point it's their responsibility to add the relevant shard(s) as dependencies.

ralsina commented 1 week ago

Unless your code uses the image formatter none of those libraries gets linked to your code AFAIK.

If they do, I will fix it.

ralsina commented 6 days ago

AFAICS, it works properly.

I tested it with markterm, which links tartrazine but uses only the Ansi formatter

This is the output of nm bin/tartrazine which does use the Png formatter and therefor has stumpy built into it:

image

And here is the output of nm bin/markterm which doesn't use the Png formatter and doesn't have any stumpy in it:

image

So, it's optional and only used when needed.

devnote-dev commented 6 days ago

I'm aware that they are not included in the binary, that would be a separate issue. My point is that optional dependencies shouldn't be installed as if they are required. This principle goes well beyond Crystal/Shards and is widely accepted in the developer space.

ralsina commented 6 days ago

Sorry, no. They are not installed anywhere except in the lib/ directory when building from source. AFAIK shards don' t provide support for "optional dependencies".

This costs the user nothing and makes exactly zero difference to anyone not using the Png writer.