Skip to content
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

display which theme is the default one in colored output #2838

Merged
merged 3 commits into from
Apr 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
- Relax syntax mapping rule restrictions to allow brace expansion #2865 (@cyqsimon)
- Apply clippy fixes #2864 (@cyqsimon)
- Faster startup by offloading glob matcher building to a worker thread #2868 (@cyqsimon)
- Display which theme is the default one in colored output, see #2838 (@sblondon)

## Syntaxes

Expand Down
12 changes: 10 additions & 2 deletions src/bin/bat/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use directories::PROJECT_DIRS;
use globset::GlobMatcher;

use bat::{
assets::HighlightingAssets,
config::Config,
controller::Controller,
error::*,
Expand Down Expand Up @@ -200,11 +201,18 @@ pub fn list_themes(cfg: &Config, config_dir: &Path, cache_dir: &Path) -> Result<
let mut stdout = stdout.lock();

if config.colored_output {
Copy link
Collaborator

@Enselic Enselic Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to cover the "not colored output" case, but I don't think you need to do that in this PR. The existing PR is clearly an improvement on its own in my view. But feel free to do it in this PR if you want to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree to implement the same feature for the "not colored output" case but I prefer to do it in another PR after this one is merged into master.

let default_theme = HighlightingAssets::default_theme();
for theme in assets.themes() {
let default_theme_info = if default_theme == theme {
" (default)"
} else {
""
};
writeln!(
stdout,
"Theme: {}\n",
Style::new().bold().paint(theme.to_string())
"Theme: {}{}\n",
Style::new().bold().paint(theme.to_string()),
default_theme_info
)?;
config.theme = theme.to_string();
Controller::new(&config, &assets)
Expand Down
18 changes: 18 additions & 0 deletions tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,24 @@ fn squeeze_limit_line_numbers() {
.stdout(" 1 line 1\n 2 \n 3 \n 4 \n 5 line 5\n 6 \n 7 \n 8 \n 9 \n 10 \n 20 line 20\n 21 line 21\n 22 \n 23 \n 24 line 24\n 25 \n 26 line 26\n 27 \n 28 \n 29 \n 30 line 30\n");
}

#[test]
fn list_themes() {
#[cfg(target_os = "macos")]
let default_theme_chunk = "Monokai Extended Light\x1B[0m (default)";

#[cfg(not(target_os = "macos"))]
let default_theme_chunk = "Monokai Extended\x1B[0m (default)";

bat()
.arg("--color=always")
.arg("--list-themes")
.assert()
.success()
.stdout(predicate::str::contains("DarkNeon").normalize())
.stdout(predicate::str::contains(default_theme_chunk).normalize())
.stdout(predicate::str::contains("Output the square of a number.").normalize());
}

#[test]
#[cfg_attr(any(not(feature = "git"), target_os = "windows"), ignore)]
fn short_help() {
Expand Down