colors in bat default theme stopped working in v661 on Windows, but worked in v643 #538
Comments
|
Can you please provide exact steps to reproduce the issue, which work OK in less 643 but not in 661? Which OS and terminal? Also, where did you get "less" from? |
Just
Windows 11 x64 using Windows Terminal with pwsh (PowerShell v7).
From |
|
It doesn't invoke less for me. Here's what I do:
But the result is that the file is printed to the console with colors, and it doesn't use "less" as far as I can tell. Maybe it needs some environment variables set? also, is "base16" the default theme? if not, how should i select it? |
|
I think it looks for it on the |
|
I'm not the original person who opened this issue, but I can provide a bit of help.
Bat will pass The README for bat has some more details about the command-line arguments being given to less, in case that influences whether this can be reproduced.
You can either pass it as the command-line arguments |
OK, I can reproduce the issue. The PATH needs to be modified so that it includes the dir where But after modifying the path (e.g. And indeed no colors at all for me with less 661, but yes colors with less 643. I think I know what's the issue, and for now the solution is to add I think the issue is that the internal SGR color processor was improved in 661, and changes how color is handled. My current hypothesis is that by default bat uses a theme with 256 colors or true colors, and when less 643 encountered unsupported colors (like 256 or true color escape sequences) then it switched to "passthrough mode" which stops processing colors internally and let the terminal handle it. And because the terminal on windows 10 can handle 256 and true colors (which I presume the default theme in "bat" uses), then it happens to work in 643. But less 661 has somewhat improved internal SGR processor, where if a color is unidentified then it produces no color rather than passthrough it to the terminal. this still needs to be confirmed, but for now that's my guess - that it worked due to a bug in less which "gave up" on trying to process the colors. The solution is, as noted, with The next version of less would hopefully greatly improve the internal color processor so that this issue should go away, but it might take a while till the next version is released. |
|
You can check here sharkdp/bat#1573 (comment) where they discuss what ANSI escapes bat uses that trigger this. |
I know. You can see that I posted there too some months ago. But this is not the same issue. The issue there was that the base 16 theme has one or two colors which use 256 colors, and these didn't work correctly with -R. But the issue here is that there are no colors at all in less with the default theme, again, I don't know what the default theme is yet, and which set of colors it uses. If you want to help, please try to figure it out and post the result here. If not, I'll figure it out at some stage on my own. |
|
The default theme is decided here (I think): https://github.com/sharkdp/bat/blob/6fc58821a53e8dff0690a1f6a144cee6d4676b71/src/assets.rs#L93, which is then taken from this submodule https://github.com/jonschlinkert/sublime-monokai-extended I will try and pipe to some hexdump and see what escapes it actually uses later if I don't forget 😛 |
A step ahead of you :) One thing to keep in mind is that I did this from Linux, but the escape sequences should be the same. This is the command used to extract the escape sequences: bat screen.c --color=always --decorations=always \
| sed 's/\(\x1B\[[^m]*m\)/\n\1\n/g' \
| grep $(printf "\x1B") \
| bat -A --decorations=never \
| sort -uWithout COLORTERM: With COLORTERM: |
|
OK, so title edit was right, because the issue is indeed not with base16 theme like the original description, but rather with the default theme, which is "monokai". I think monokai ends up using this file, but I'm not 100% sure: https://github.com/jonschlinkert/sublime-monokai-extended/blob/0ca4e75291515c4d47e2d455e598e03e0dc53745/Monokai%20Extended.tmTheme Regardless, it does output 256 colors, and indeed the behavior where "less" gives up on unsupported sequences - which include 256 colors - was changed between less v650 and v651. Specifically, at commit f9f2498 . So the hypothesis I posted earlier is confirmed and correct, and the solution which I posted earlier is also still correct (setting the arguments which "bat" uses with "less" to to The solution was also posted by myself on April at the "bat" tracker sharkdp/bat#1573 (comment), as well as more recently at the same issue by @segevfiner. It should be noted that there's no issue with themes which only use 8/16 colors, like the "ansi" theme. It only happens with 256 and true colors. The core issue is that the internal SGR processor doesn't yet support 256 or true colors, and The difference between 643 and 661 is how unsupported sequences are handled. And, as noted previously, we do have plans to improve it in the next release, but we don't know when that would happen. |
|
We could have bat conditionally pass There is precedent for changing the arguments based on less's version. It was done in the past with |
No version of "less" supports 256/true colors on any version of windows. The difference is in how unsupported sequences are handled. Previously it gave up and did passthrouh, now it identifies it and ignores this sequence. As far as I can tell I'm guessing its behavior remained the same or similar enough. But it would only work on windows 10 and later. It would definitely break in windows 7 (which I don't know if bat supports) because In the next release the internal SGR processor would support 256 and true colors also on windows 7 - by translating them to 8/16 colors which the terminal supports, and hopefully will passthrough to the terminal by default if the terminal supports it. @gwsw commit f9f2498 is not big, but if we revert it then 256 colors break on win7 etc. Maybe we can push a small commit and release quickly a new version where it ignores unsupported sequences when VT is disabled, or passthrough the rest of the line when VT is enabled? (I can open a PR). Basically the current behavior on win7, and the previous behavior on win10. Then there would only be one public version (661), hoepfully short lived, where 256 "don't work by default" on windows 10+ ? Hopefully we can do that before you start adding new features on master after v661. (and then we can add the bigger change of properly supporting it later, without hurry) |
Actually, on win7 So unless I'm missing something, there should no issue in always adding However, I'd prefer that "bat" doesn't add |
Sorry for the confusion there, I meant supported as in working from the perspective of a user.
I agree. I was under the impression that we would have to wait until less fully supports the 256-color and truecolor sequences, but if it's possible to issue a small update on less's end, that would be preferable to adding a workaround on bat's end. |
Well, it does look as if it's working, but there are subtle issues, like that the default transformations which "less" does on windows stop working (like changing underline with default colors to cyan), and also that the internal SGR processor loses track of the terminal's state (because it stopped parsing the SGR sequences till the end of the line, but these sequences were sent to the terminal), and this can result in other incorrect output. However, I agree that subjectively it can be preferrable on windows 10+, even if somewhat buggy, and it might even be possible to reduce the resulting issues with some additional minimal change. I'll try to open a PR ASAP to address this and see how much I can do safely and minimally for a quick release, but the desicion is ultimately by @gwsw . |
"sgr mode" (-Da) bypass the internal SGR processing and passthrough everything to the terminal directly, which works fine on win10+ where the terminal supports more sequences than us - like 256 colors. But by default (without -Da), we do internal SGR processing and apply the values to the console outselves, including some transformation of -D<x> values unique to windows (like underline to cyan). Before commit f9f2498 (which is just before v651), unknown SGR values resulted in aborting the SGR parsing and instead passthrough till the rest of the buffer to the terminal directly. This had broken output if the terminal doesn't support VT (e.g. win7), but seemingly correct output with VT, e.g. 256 colors on win10. It did have some issues still even with VT, like losing track of the terminal state, so if the next buffer has e.g. SGR 1 (bold) then we'd output a wrong color (the last state we parsed, plus bold). It also stopped applying the various -D<x> config values till the end of this buffer, because this buffer was switched to passthrough. Commit f9f2498 changed this process, by ignoring only the rest of the current sequence if an unknown value is encountered, so that the next sequences in the buffer are still processed normally. Additionally, we then never lose track of the terminal state, because either we recognize a value and apply it, or we do neither of those. Specifically, we never apply a value which we can't recognize/track. While it fixed the junk output when VT is disabled, it also had an effect which made 256 and true colors stop "working" on win10+. So this commit brings back this behavior of "working" 256/true colors on win10+ by default, but a bit more refined than before f9f2498 . When VT is disabled then there's no change, and like before - no junk output from unknown sgr values. When VT is enabled and an unsupported SGR value is encountered, we stop applying the SGR values just like before, and also keep parsing just like before. However, instead of applying the values which we did recognize, we now switch to a mode which passthrough the whole sequence - because the terminal can handle it, and we know we've lost state sync. Then, we keep parsing sequences for hints but also keep passthrough the sequences until we know what the terminal state is - after reset. That is, the scope of "unknown terminal state and no -D<x>" is now minimal and exact (till first reset), instead of till the first reset at the next buffer or later. This difference it non negligible, because many times a buffer is a full line - which ends in a reset. So now we identify this reset so that the next line is in sync, but before commit f9f2498, the sgr state at the next line was the one we had before we passthrough the rest of the buffer, so the whole next line could be wrong. Fixes gwsw#538
|
@eth-p if you want to try out a binary of v661 plus the commit at #539 , I'd appreciate any feedback: This was built using llvm-mingw 18.1.7, linked with UCRT (rather than MSVCRT). |
|
@avih My only Windows machine is Windows 11 so I can't speak for earlier versions of the OS, but that looks considerably better! |
Thanks. Are you observing any issues? or is that an understatment of "perfect as far as I can tell"? There should be at least one additional difference to 643 in "bat", and that's that the default charset is now UTF-8, so the boxdraw lines which "bat" generates for borders/separators/etc look correct in 661 (and same in the attached zip), but broken in 643. Also, the status line is now normal inverse in v661, instead of blue/white in v643 (and there are more additional fixes which are not apparent immediately) |
The latter, sorry. On my machine, the unpatched 661 release did not appear to have any colors at all, whereas the patched version you provided looked as I expected it to. @segevfiner, as the one who reported the issue, does the patch solve it for you as well? |
"sgr mode" (-Da) bypass the internal SGR processing and passthrough everything to the terminal directly, which works fine on win10+ where the terminal supports more sequences than us - like 256 colors. But by default (without -Da), we do internal SGR processing and apply the values to the console outselves, including some transformation of -D<x> values unique to windows (like underline to cyan). Before commit f9f2498 (which is just before v651), unknown SGR values resulted in aborting the SGR parsing and instead passthrough till the rest of the buffer to the terminal directly. This had broken output if the terminal doesn't support VT (e.g. win7), but seemingly correct output with VT, e.g. 256 colors on win10. It did have some issues still even with VT, like losing track of the terminal state, so if the next buffer has e.g. SGR 1 (bold) then we'd output a wrong color (the last state we parsed, plus bold). It also stopped applying the various -D<x> config values till the end of this buffer, because this buffer was switched to passthrough. Commit f9f2498 changed this process, by ignoring only the rest of the current sequence if an unknown value is encountered, so that the next sequences in the buffer are still processed normally. Additionally, we then never lose track of the terminal state, because either we recognize a value and apply it, or we do neither of those. Specifically, we never apply a value which we can't recognize/track. While it fixed the junk output when VT is disabled, it also had an effect which made 256 and true colors stop "working" on win10+. So this commit brings back this behavior of "working" 256/true colors on win10+ by default, but a bit more refined than before f9f2498 . I.e. in v643 it appeared that 256 colors work by default, but in v661 it appeared to have been broken, and now it's fixed. When VT is disabled it's now the same as v661, and like in v661, it also doesn't have junk output from unknown sgr values. When VT is enabled and an unsupported SGR value is encountered, we stop applying the SGR values just like in v661, and also keep parsing just like in v661. However, instead of applying the values which we did recognize like in v661, we now switch to a mode which passthrough the whole sequence, because the terminal can handle it, but we know we've lost sync. Then, we keep parsing sequences for hints but also keep passthrough the sequences until we know what the terminal state is - after reset. That is, the scope of "unknown terminal state and no -D<x>" is now minimal and exact (till first reset), instead of till the first reset at the next buffer or later, and also we don't get hit by wrong color when we've lost track with the terminal state. This difference it non negligible, because many times the buffer is a full line - which ends in a reset. So now we identify this reset so that the next line is in sync, but in v643 the sgr state at the next line was the one we had before we passthrough the rest of the buffer, so the whole next line could be wrong. Fixes gwsw#538
My current thinking is, since less-661 was a very large release with many changes, I would not be surprised if there are some bugs reported over the next few weeks. I plan to leave the master branch unchanged except for bug fixes for a while, perhaps a month or two, and will do a bug fix release sometime in that time frame, depending on the severity of the bugs found. I don't plan to merge any new features from the post659 branch into master until after the bug fix release on master. |
|
For future reference:
The rest below is about the default behavior, without Up to and including v643 (specifically v650):
This produced completely broken output for any 256/true colors sequences where all values were under 50, like However, because many 256/true colors seqences do include values of 50 or more, these appeared to work correctly on win10+, as they were passthrough to the terminal. Additionally, due to losing the terminal state with 256/true colors, further wrong output are still possible. Specifically, and case in point, all the values at the monokai theme in bat (which is quite bright) include values of 50 or more, hence it seems working as if using But this is only by luck. Other themes are likely to be at least partially broken, and the darker/dimmer the theme, the more likely it is to have more broken colors (which don't include values of 50+). v661 is much simpler:
This means that 256/true colors never work in v661, because 38/48 abort the current sequence and without any passthrough to the terminal. Whether v661 better or worse than v643 is subjective. IMO v661 is better, because it's 100% consistent - 256/true colors don't work, never produce incorrect output (by misinterpreting their values), never look like they sometimes work, and never carry incorrect terminal state forward. While v643 is inconsistent because some 256/true colors seem to work, while others don't work and instead are misinterpreted, and can also create future currptions due to corrupted state. So by this summary, I now think there's no rush to release a "fix" because it was never working reasonably in v643 to begin with, despite appearling mostly working with the "bat" default theme. @eth-p agreed? Additionally, we do have plans (and code) to greatly enhance the internal SGR processor so that it does support 256/true colors. However, this is not a quick fix release, and instead it should be ready for the next proper release. All that being said, I think we do have an opportunity for a big win with little risk in a followup fixup release, and that's to combine the "good" parts of v643 and v661 and behave like It builds on the stability and consistency of v661, and when it detects an unsupported SGR value, it switches to a "controlled" passthrough mode, where it keeps passthrough sequences until it detects a valid reset sequence, and then switches back to the SGR processor. This means that all 256/true color sequences work, and the state is effectively never corrupted (it does get out of sync, but we know it and don't use it in such case), thus avoiding any incorrect colors after 256/true colors sequences. @gwsw after observing the patch of #539 , do you agree this should be included in the next fixup release, despite being a (cheap but great) enhancement rather than a fix? (and I'll need to revise the commit message, because v643 is actually quite worse than what the commit message suggests) |
|
I haven't tried to analyze the patch in detail, but on cursory review and based on your explanation above, it seems reasonable to me to include it in the next bug fix release. |
To get 256/true colors working with -R (win10+ only), "sgr mode" (-Da) should be used to bypass the internal SGR processing and passthrough everything to the terminal directly - which can handle it on win10+. "sgr mode" was and is working well for some releases now. But by default (without -Da), we do internal SGR processing and apply the values to the console outselves, including some transformation of -D<x> values unique to windows (like underline to cyan). In the previous release v643, the default behavior (no -Da) was: - ANSI 8 colors SGR and attributes (1 for bold, etc) work well. - Unknown values under 50 (like 38, 48) were processed as 0 (reset). - SGR values of 50 or more aborted the internal SGR processor and switched to passthrough till the end of the current input buffer. - On abort+passthrough, the internal SGR state was kept unmodified. This resulted in broken behavior for 256/true colors, e.g.: - SGR 38;5;33 was incorrectly applied as reset, blink, yellow-fg. - SGR 38;5;50 was passthrough, and the internal state became blink. The passthrough was clearly broken on pre-win10, as the terminal can't handle it, but on win10+, the passthrough seems to work if not looking too closely. E.g. in some syntax highlighters where all the 256/true colors include at least one value >= 50, it did appear working on win10+. In release v661 the behavior was simplified and tightened: - ANSI 8 colors SGR and attributes (1 for bold, etc) work well. - Any unknown value (like 38, 48) aborts the rest of this sequence without passthrough (prior values at this seq, if any, do apply). As a result, 256/true colors sequences are ignored, there's no issue on pre-win10, we never misinterpret such sequences, never passthrough, and the internal state is always in-sync with the actual terminal. However, it does look like a regression compared to cases which did appear to be working in v643. This commit fixes this "regression", but without the downsides of v643: - On win10+, instead of aborting the sequence on unknown values like v661 does, we now switch to "controlled passthrough" mode until reset is detected, and then switch back to the internal SGR handler. As a result: - On pre-win10 it's like v661 (256/true colors are ignored cleanly). - On win10+: all 256/true colors always "work", never misinterpreted. - The internal state is in-sync - cleared on reset after passthrough. - Like -Da, the -D<x> transformations are skipped during passthrough. Fixes gwsw#538
Since the v651 release of less, bat colour support has been broken because less stopped passing unknown colour codes and instead removed them. This broke `truecolor` support. When less >v663 is released this will be fixed and the `BAT_PAGER` env variable can be removed. - gwsw/less#539 - gwsw/less#538
See sharkdp/bat#1573
Colors printed by
batusing just-Rused to work in less v643, but no longer do in v661 without-Dawhich was suggested as a workaround.The text was updated successfully, but these errors were encountered: