Need a more sustainable way to manage known syntax mappings

[cyqsimon]

Currently everything is done in src/syntax_mapping.rs using bespoke code for each syntax mapping we want to add, which is starting to become a problem.

The SyntaxMapping::builtin function is already 160 lines long, and is nowhere near complete. As a daily user of bat, on average I run into this situation several times a week when I'm thinking, "man, this well-known file path really should have syntax mapping built in". But then I remember how it's implemented and think, "I really don't want to make the maintainability any worse than it already is".

What I'm saying is, there has to be a better way to code this to make it more maintainable and extensible (specifically, easier to add more mappings and harder to accidentally add duplicates). So I would like to ask the community for your opinion (how to organise the mappings, what patterns/libraries to use, etc.). I'm happy to help implement the changes if we come to some technical consensus.

[keith-hall]

I imagine some kind of TOML file for example could work, where it would be possible to have a list of glob to target mappings, where environment variables would be replaced and the list deduplicated before being processed sequentially.
I'm not sure how much it would help with preventing from accidentally adding duplicates, unless by some form of convention the globs would generally be ordered like longest prefix first, then lexicographically or something. Or maybe grouping by mapping target could also somehow help?
But the env var replacement and deduplication could help with replacing the current gitconfig logic.

[keith-hall]

I guess we'd be okay with losing the command line argument support for manipulating mappings if we have them in a separate config file. But presumably we'd want to merge the default mapping with the user one or something...

[cyqsimon]

I imagine some kind of TOML file for example could work, where it would be possible to have a list of glob to target mappings, where environment variables would be replaced and the list deduplicated before being processed sequentially. I'm not sure how much it would help with preventing from accidentally adding duplicates, unless by some form of convention the globs would generally be ordered like longest prefix first, then lexicographically or something. Or maybe grouping by mapping target could also somehow help? But the env var replacement and deduplication could help with replacing the current gitconfig logic.

Yeah my mind went to a similar place. I'm thinking, maybe instead of having a single file, we can have a list of files, where each one specifies mapping for a particular application and/or use case? This will help to reduce the possibility of introducing duplicates I think. So for example syntax_mapping_defs/linux/systemd.toml would specify all the various unit file paths.

Then we can use phf_codegen to build this info into the binary at compile time. We can even do fancier things like switch on the target OS to determine which mappings should be included. Deduplication checks can happen here too.

Then finally at runtime we apply your env replacement logic, and all (hopefully) should be nice and fast.

I guess we'd be okay with losing the command line argument support for manipulating mappings if we have them in a separate config file. But presumably we'd want to merge the default mapping with the user one or something...

Yeah I don't think merging the mapping is going to be much of a challenge. No need to lose functionality here.

[cyqsimon]

Also, is TOML the most appropriate format choice for this? Not that I have any preference for or against any markup format (well, unless we're considering XML I guess); this is just to have all bases covered.

[keith-hall]

I guess TOML came to my mind first because it's already used by Cargo, so Rust developers would be familiar with it. I personally wouldn't want JSON because it is too strict with no support for comments or trailing commas, but I could always be overruled here 😉 I'd personally be okay with YAML 1.2 though, but I know it can be hard for newcomers.

Thinking about it may also make sense to continue to support the current config file/command line arg support for a while to give users a chance to migrate without suddenly breaking their workflows.

As a daily user of bat, on average I run into this situation several times a week when I'm thinking, "man, this well-known file path really should have syntax mapping built in". But then I remember how it's implemented and think, "I really don't want to make the maintainability any worse than it already is".

Btw I hope you are keeping track of these so we can get them added once the new solution is in place 😉

[liamdiprose]

Would https://github.com/tree-sitter/tree-sitter be suitable?

[cyqsimon]

Not sure how tree sitter would help. This syntax mapping we are addressing here is not related to content-based syntax detection, rather path-based syntax override.