-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Please support .debdiff files #2940
Comments
If nobody is picking this up, I might have a go. Any hints on how it should be approached? |
Hi, I believe the way to go is to add a syntax mapping for it, which is done with TOML files - see https://github.com/sharkdp/bat/tree/master/src/syntax_mapping/builtins |
@bdrung Presumably you'd want |
@jacg I cannot remember seeing |
Are there any guidelines on what to use as a test case file? Curiously enough, if I copy However, if I use the the diff from the random NMU bug mentioned above, then In other words, for some diff files, |
Adding a file # .debdiff is the extension used for diffs in Debian packaging
[mappings]
"Diff" = ["*.debdiff"] seems to do the job. However:
This makes me wonder whether it could/should be done using the same mechanism that was used to recognize the |
It can be done, yes, you'd need to patch the
Presumably the |
Is either approach to be preferred over the other? (The toml file approach certainly looks simpler.) As for testing, I started off by trying to generate a test failure, as a sanity check, by modifying the contents of |
I would think it's generally easier to maintain the TOML mappings than a series of patches. The TOML mappings were something added fairly recently, for reference. Previously mappings were maintained in Rust code, which wasn't so fun to modify. If we want simple file extension globs to show up differently in I just tried changing
|
Thanks, I can see the tests picking up changes to the
|
As with any reasonably complex software, the language tooling's simple test suite often doesn't cover everything. Generally, one can look at the CI steps performed for an exhaustive list of what commands are executed for verification that everything is working as expected. The regression test for example is defined at bat/.github/workflows/CICD.yml Line 112 in 424c02d
I would personally say "yes" - it shouldn't have been possible to merge changes which would fail these tests. Indeed, the latest CI run on
As per the previous answer, "yes". What environment (OS etc.) are you running |
FWIW, that's why I try to stick a justfile at the root of my projects, with an obvious and easily-discoverable recipe that runs all the tests. Admittedly, this can diverge from what is executed in CI workflows, so it's not foolproof.
Linux (NixOS). Not sure what other environment details would be useful to mention. Tried it in a bare bones They seem to be mostly caused by the odd token having an unexpected colour. Anyway, for the time being I can work with this small number of tests failing locally, and rely on CI (which is working fine in my draft PR (#2947) to catch anything I miss. |
Ah, sorry, I see that the link to the draft PR in the previous comment was pointing at the issue that inspired it, rather than the PR itself. Here is the correct link: #2947 (now also fixed in the comment). |
Debian source diff files are often named
.debdiff
. They are normal diff files. Please support coloring them by just treating them as diff files.The text was updated successfully, but these errors were encountered: