--list-themes + BAT_OPTS can be problematic

[gurgeous]

trafficstars

Thank you so much for creating bat! Very happy with the app.

I'm customizing things a bit and my BAT_OPTS is breaking --list-themes. It's paging one theme at a time and the theme names are not shown. That makes it unusable, unfortunately. Maybe we should ignore --paging=always (or perhaps all config) when running that command?

BAT_OPTS="--theme=TwoDark --paging=always --pager='less -j3 -i -J -M -R -S -w -z-4'"

Software version

bat 0.18.0

Operating system

Darwin 19.4.0

Command-line

bat --diagnostic 

Environment variables

SHELL=/bin/zsh
PAGER=less
BAT_PAGER=<not set>
BAT_CACHE_PATH=<not set>
BAT_CONFIG_PATH=<not set>
BAT_OPTS="--theme=TwoDark --paging=always --pager='less -j3 -i -J -M -R -S -w -z-4'"
BAT_STYLE=<not set>
BAT_TABS=<not set>
BAT_THEME=<not set>
XDG_CONFIG_HOME=<not set>
XDG_CACHE_HOME=<not set>
COLORTERM=truecolor
NO_COLOR=<not set>
MANPAGER=<not set>

Config file

Could not read contents of '/Users/amd/.config/bat/config': No such file or directory (os error 2).

Compile time information

• Profile: release
• Target triple: x86_64-apple-darwin
• Family: unix
• OS: macos
• Architecture: x86_64
• Pointer width: 64
• Endian: little
• CPU features: fxsr,sse,sse2,sse3,ssse3
• Host: x86_64-apple-darwin

Less version

> less --version 
less 487 (POSIX regular expressions)
Copyright (C) 1984-2016  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.
Homepage: http://www.greenwoodsoftware.com/less

[sharkdp]

Thank you for reporting this. That should be fixed.

To reproduce this, run:

BAT_OPTS="--paging=always" bat --list-themes

or simply

bat --paging=always --list-themes

[mohamed-abdelnour]

Hello, I am currently trying to work on this and wanted to discuss the implementation. I can think of two routes to take in solving this:

• Force PagingMode::Never if --list-themes is passed to bat. This is fairly straightforward, and I have already prepared a basic implementation here: mohamed-abdelnour/bat@74ecd925b24bd820a0def7507ac8af769b24c418. If we settle on this, I can add a few tests and create a PR. The problem with this is that it adds to the inconsistency mentioned in #1587.
• Add a way for bat to properly support paging with --list-themes. I am not too familiar with the codebase, but as far as I can tell, this would be significantly more involved given the current implementation of Controller. I'm not sure if it would be worth it to make significant changes for the sake of --list-themes alone, but this would theoretically address #1587.

Which approach do you think is more preferable, or do you have a completely different solution in mind? I will be waiting for your feedback!

[cuberoot74088]

Hello, I was also just looking into #1587 to get started ;-)

I tried to follow the idea/implementation in #1394 but stumbled across the same problem with the Controller - as far as I can see, it will print multiple inputs with a single config, but in this case you would have to change the config.theme as you go through the list of themes and correspondingly formatted demo code snippets, right?

I thought of collecting pre-formatted Strings per theme and put this into the controller to get into paging, but could only write to Stdout so far and the Controller just returns a bool.

Looking forward to some ideas and comments :-)

[mohamed-abdelnour]

Update!

• I updated the first implementation to use the bat_warning macro instead of manually defining a style for the warning. Here is a link to the latest commit from that branch: mohamed-abdelnour/bat@1ad5daa6d92ac99107cd26e737b36958df876f88.
• I started working on the second implementation; it's now working properly, as far as I can tell! However, I have not written any tests just yet, and I have never written anything like this before, so I may be missing something. I just wanted to share my progress on this and get feedback, so here is a link: mohamed-abdelnour/bat@d1b9a68aae1cc1426fcda86925843af95fe3974c.

Overall, I think the second one is probably a little nicer, because:

• it gives the option to set the PagingMode;
• it makes #1587 quite simple to fix;
• as @eth-p mentioned in #956, having an extra OutputType is a good first step towards implementing the Display trait for PrettyPrinter, so it might help us out there, too. I have not tried to work on that just yet, though.

It's very amusing how seemingly related the issues labelled "good first issue" are! :)

Looking forward to getting your feedback on this!

[lucatrv]

I confirm this issue is still open as of bat 0.20.0 (0655ecf2).