search

sharkdp / bat

A cat(1) clone with wings.
Apache License 2.0
49.02k stars 1.23k forks source link

base16 theme not working correctly with `less -R` #1573

Open jcwillox opened 3 years ago

jcwillox commented 3 years ago

Describe the bug you encountered:

The base16 and base16-256 themes do not work correctly when using less -R. I've tested this in PS5, PSCore (using Windows Terminal, and the native conhost terminals) and cmd.exe and they all have the same issue.

It turned out the issue was caused by bat using less -RF and setting BAT_PAGER to less -rF fixed the issue.

This issue seems to be more of an issue with running less on Windows, as the same version (but for linux) of less running in git-bash doesn't have this issue.

More generally, when using less on Windows it would help to set $env:LESS = "-r" so less accepts colours by default.

image

What did you expect to happen instead?

image

How did you install bat?

scoop

bat version and environment

Software version

bat 0.18.0

Operating system

Windows 6.2.9200

Command-line

"C:\\Users\\Josh\\scoop\\apps\\bat\\current\\bat.exe" .vuerc --diagnostic

Environment variables

SHELL=<not set>
PAGER=<not set>
BAT_PAGER=<not set>
BAT_CACHE_PATH=<not set>
BAT_CONFIG_PATH="C:\\Users\\Josh\\.config\\bat\\config"
BAT_OPTS=<not set>
BAT_STYLE=<not set>
BAT_TABS=<not set>
BAT_THEME=<not set>
XDG_CONFIG_HOME=<not set>
XDG_CACHE_HOME=<not set>
COLORTERM=<not set>
NO_COLOR=<not set>
MANPAGER=<not set>

Config file

# This is `bat`s configuration file. Each line either contains a comment or
# a command-line option that you want to pass to `bat` by default. You can
# run `bat --help` to get a list of all possible configuration options.

--italic-text=always
--map-syntax ".ignore:Git Ignore"
--map-syntax ".vuerc:JSON"
--style="numbers,changes,rule"
--theme="base16"

Compile time information

Less version

> less --version
less 563 (Spencer V8 regular expressions)
Copyright (C) 1984-2020  Mark Nudelman

less comes with NO WARRANTY, to the extent permitted by law.
For information about the terms of redistribution,
see the file named README in the less distribution.
Home page: http://www.greenwoodsoftware.com/less
sharkdp commented 3 years ago

Thank you for reporting this. Would you mind testing this with the latest less version? https://github.com/jftuga/less-Windows

jcwillox commented 3 years ago

Sure, just tested it with the latest version of less and unfortunately it still produces the same result

sharkdp commented 3 years ago

@mk12: Only if you find the time and in case you are interested: could you take a look at this? Do you know why these themes could cause problems on Windows (using less -R)?

mk12 commented 3 years ago

Summary

less -r passes through all control characters, whereas less -R only allows ANSI color escape sequences and OSC 8 hyperlink sequences. I'm guessing that less -R is being too conservative here.

@jcwillox, could you test the following:

  1. Does this have color? printf '\x1b[91mBRIGHT RED\x1b[0m' | less -R
  2. Does this have color? printf '\x1b[38;5;9mBRIGHT RED\x1b[0m' | less -R
  3. Does echo $LESSANSIMIDCHARS print anything?

Details

Here are the escape sequences we use:

As noted in terminal.rs, ansi_term doesn't support emitting the bright colors via the 90-97 and 100-107 codes, so we use Fixed instead which produces the 38;5/48;5 codes:

https://github.com/sharkdp/bat/blob/3aea51455b8acf553f4481996f93d7e219130c2b/src/terminal.rs#L13-L33

The screenshots show false and true not being highlighted, which use bright red in the base16 theme. So my hunch is that less -R is not passing through \x1b[38;5;9 (but possibly would pass through \x1b[91m).

If we fix ansi_term to allow 90-97/100-107 codes, it would enable support for terminals that have bright colors but not full 256-bit color, and possibly fix this issue. But only for base16. The base16-256 theme makes 8-15 identical to 0-7, and instead shoves the extra colors in 16-21, which can only be set using the 38;5/48;5 codes.

From less.man in the source for less 581.2:

-R or --RAW-CONTROL-CHARS
       Like -r, but only ANSI "color" escape sequences and OSC 8 hyper-
       link sequences are output in "raw" form.  Unlike -r, the  screen
       appearance  is  maintained correctly, provided that there are no
       escape sequences in the file other than these  types  of  escape
       sequences.   Color  escape sequences are only supported when the
       color is changed within one line, not across  lines.   In  other
       words,  the beginning of each line is assumed to be normal (non-
       colored), regardless of any escape sequences in previous  lines.
       For the purpose of keeping track of screen appearance, these es-
       cape sequences are assumed to not move the cursor.

       OSC 8 hyperlinks are sequences of the form:

            ESC ] 8 ; ... \7

       The terminating sequence may be either a BEL character  (\7)  or
       the two-character sequence "ESC \".

       ANSI color escape sequences are sequences of the form:

            ESC [ ... m

       where  the "..." is zero or more color specification characters.
       You can make less think that characters other than "m"  can  end
       ANSI  color escape sequences by setting the environment variable
       LESSANSIENDCHARS to the list of characters which can end a color
       escape  sequence.   And  you can make less think that characters
       other than the standard ones may appear between the ESC and  the
       m  by  setting  the environment variable LESSANSIMIDCHARS to the
       list of characters which can appear.

And from line.c:

    mid_ansi_chars = lgetenv("LESSANSIMIDCHARS");
    if (isnullenv(mid_ansi_chars))
        mid_ansi_chars = "0123456789:;[?!\"'#%()*+ ";

So it would seem 38;5/48;5 codes should be let through by default.

jcwillox commented 3 years ago
  1. Yes
  2. Interestingly this is not coloured in less
  3. Empty

So it would seem that less isn't handling ANSI-256 correctly on Windows, but ANSI-16 works fine.

Another very broken example this "\x1b[38;5;33mIM BLUE\x1b[0m" which produces image

instead of image

avih commented 6 months ago

This issue still exists with bat 0.24 and latest less for windows (including the master branch).

For reference, the issue is that the base16 theme produces 256-colors escape sequences like \e[38;5;8m, but less -R on Windows uses an internal SGR processor (to translate SGR to windows colors), and this doesn't currently support 256-color sequences. It also doesn't currently support the bright-only AIX-term SGR values of 9x and 10x, like \e[93m.

There exists a solution on windows 10+ only, by using less -R -Da. This bypass the internal SGR processor, and passthrough the escape sequences directly to the terminal, which, on windows 10 and later, does support 256-color sequences (as well as SGR 9x and 10x).

Using less -r (instead of less -R -Da) is not recommended, as it makes it harder for less to calculate the line length, and also passthrough non-SGR escape sequences, which is typically undesirable.

There's a plan to improve the SGR processing on Windows with less -R so that -Da would not be required, while also accepting a wider range of SGR sequences (including 256-color, AIX-term bright-only, and more), but for now it only supports attributes (bold, italic, etc) and the 8 colors in 3x and 4x.

However, IMHO the bigger issue is not specific to less on Windows, but rather that the base16 theme produces 256-color sequences, and I'd think it's preferable if it only used the normal 8 colors (SGR 3x, 4x) and bold (and preferably avoid italic/underline, because not all terminals support it, and it also kinda doesn't fit conceptually with what sounds like a basic 16 colors theme).

This would make the base16 requirement much more basic, and would guarantee it works on a wider range of terminals.

As far as I can tell, after only a cursory examination of base16 output, it only uses \e[38;5;8m and \e[38;5;9m (bright black and bright red), which should work similar enough with \e[30;1m and \e[31;1m, respectively (and similarly, replacing all the other 38;5;<8-15> with 1;3<0-7>, assuming it only applies to fg colors, or also 1;4<0-7> if it needs bg too).

It's "similar enough" and not necessarily identical for two reasons:

Would you consider simplifying the base16 theme so that it only produces SGR 1, 3x, 4x ?

Or maybe create a variant of it which adheres to these limitations (ansi16?)?

Or maybe add an option to ensure that the output only uses such sequences?

mk12 commented 6 months ago

@avih We can’t reduce base16 to fewer colors because bat doesn’t define base16, it’s an existing spec. However, bat already has another theme called ansi which is exactly what you want, unless I’m missing something. Did you try that?

avih commented 6 months ago

bat already has another theme called ansi which is exactly what you want, unless I’m missing something. Did you try that?

Right. I have been using base16 (with the conversion hack below) for a long while, and have either forgot about the ansi theme, or missed it. ansi is indeed a good solution, which does look similar to base16, though comments are in green, while base16 has gray comments, which subjectively I find nicer (I'm sure one could customize a theme, but I haven't tried).

Nevertheless, the ansi theme should be a good alternative.

We can’t reduce base16 to fewer colors because bat doesn’t define base16, it’s an existing spec

Right. I wasn't aware of it being a spec. That being said, at least in my use cases, and as far as I noticed, the base16 theme does produce nearly pure ansi output (SGR 1, 3x, 4x) with the exception of bright black and bright red which use 256-color sequences (to use a bright variant of the normal 8 colors).

For future reference, this sh script invokes bat with base16 theme and converts the bright-8-as-256-sequence which base16 produces, to bold ansi sequences, and I've not noticed issues with it (sh and sed can be found in msys2, git-for-windows, busybox-w32, and others):

bat-base16-ansi.sh ```sh #!/bin/sh # # bat-base16-ansi: use bat with base16 theme but produce ansi output. # # The "base16" bat theme produces nearly standard ANSI output (SGR 1, 3x, 4x), # except that it uses 256 color sequences for bright variant of some of the # normal 8 colors, so convert those to bold ANSI colors. # # Note that this is intended for non-interactive output, like a pipe # # E.g. bat-base16-ansi -p file.c | less -R # convert bright 8 colors as 256-color sequences to bold ansi sequences: # \e[<3/4>8;5;<8-15>m -> \e[1;<3/4><0-7>m c256to16() { ESC=$(printf \\033) for x in 8 9 10 11 12 13 14 15; do set -- "$@" -e "s/$ESC\[\([34]\)8;5;${x}m/$ESC[1;\1$((x-8))m/g" done sed "$@" } bat -f --theme=base16 "$@" | c256to16 ```

And as a reminder about the original issue of base16 with less -R on windows: use less -R -Da (works on win10+) to solve the issue, and hopefully in the future -Da would not be required.

segevfiner commented 3 months ago

To put it shortly I set:

$env:BAT_PAGER="less -FR -Da"

As a workaround for now, until less is updated to support those escape sequences without -Da, which I'm not entirely sure what it does...

segevfiner commented 3 months ago

This doesn't happen with less v643, but does with v661. It seems like something broke between those versions in regards to the base16 colors.

EDIT: Submitted https://github.com/gwsw/less/issues/538

NathanielJS1541 commented 3 weeks ago

Just to update this issue, this has been fixed as of less v663. However, the current release is still v661, so might be a while until the fix is visible to users.

I did discover that there is a versions/less-beta package on scoop, but it is currently an exact mirror of main/less, since the GitHub repo it pulls from does not build the beta releases.

There is a patch written in an issue for jftuga/less-Windows that will allow it to build the beta versions of less, but this was opened in 2022 and has seen no activity since.

avih commented 3 weeks ago

There is a patch written in an issue for this if you want to build it yourself.

If one builds it themselves, then just build the master branch, which includes the fix.

A new public release (which includes the fix) should hopefully happen very soon now, as since v661 there have already been few public betas (which also include the fix).

It is worth noting though that it's more than a "fix".

v643 was actually very broken by default with 256 and true color sequences, but happened to work nearly perfectly on windows 10, but only if the theme SGR values didn't have any values under 50. And on win7 it was completely broken regardless.

v661 made 256/true colors simply unsupported and officially ignored by default on all versions of windows.

However, using -Da (in v661, v643, and many earlier revisions, and future ones too) does make it work on win10 where the terminal supports 256/true colors.

The "fix" makes 256/true colors work correctly on win10 by default, even with SGR values below 50, and they're also correctly ignored on win7/8. I.e. it's actually much better than v643 on all versions of windows.

The next-next release would hopefully make 256/true colors work also on win7/8 by translating these colors internally to 8/16 colors - which win7/8 console does support.

FYI.

NathanielJS1541 commented 3 weeks ago

If one builds it themselves, then just build the master branch, which includes the fix.

Sorry, I explained that badly. The patch I mention is for the less-Windows repo that is linked in the less downloads page for Windows Binaries. The linked repo does not currently contain any beta releases, so there are no Windows binaries linked on the less downloads page which contain the fix introduced in less v663 or later.

I am just pointing out that there is an open issue in the less-Windows repo which would allow it to build beta releases of less, in the hopes that it will see some attention and be merged so that windows binaries for beta versions of less are easily available. I have updated my comment to clarify this.

avih commented 3 weeks ago

There is a patch written in an issue for jftuga/less-Windows

The patch link doesn't work for me, but this works: https://github.com/jftuga/less-Windows/issues/12