sharkdp/bat

Option to change the git signs

Opened this issue · 10 comments

Is it possible to add an option to change the git signs?

@sand4rt I can create a PR for this feature request if @sharkdp approve it !

Sneak peek:

Image
  1. Modern gitsigns: matching nvim gitsigns plugin(https://github.com/lewis6991/gitsigns.nvim)
  2. Updated printer.rs too to read the markup.changed, markup.inserted, markup.deleted scopes, otherwise just use the default styles Yellow.normal(), Green.normal(), Red.normal():
impl Colors {
    fn plain() -> Self {
        Colors::default()
    }

    fn colored(theme: &Theme, true_color: bool) -> Self {
        let gutter_style = Style {
            foreground: match theme.settings.gutter_foreground {
                // If the theme provides a gutter foreground color, use it.
                // Note: It might be the special value #00000001, in which case
                // to_ansi_color returns None and we use an empty Style
                // (resulting in the terminal's default foreground color).
                Some(c) => to_ansi_color(c, true_color),
                // Otherwise, use a specific fallback color.
                None => Some(Fixed(DEFAULT_GUTTER_COLOR)),
            },
            ..Style::default()
        };

        let scope_style = |scope_name: &str, default_style: Style| {
            theme
                .scopes
                .iter()
                .find(|item| {
                    item.scope
                        .eq(&ScopeSelectors::from_str(scope_name).unwrap())
                })
                .and_then(|item| item.style.foreground)
                .map(|c| Style {
                    foreground: to_ansi_color(c, true_color),
                    ..Style::default()
                })
                .unwrap_or(default_style)
        };

        Colors {
            grid: gutter_style,
            rule: gutter_style,
            header_value: Style::new().bold(),
            git_added: scope_style("markup.inserted", Green.normal()),
            git_removed: scope_style("markup.deleted", Red.normal()),
            git_modified: scope_style("markup.changed", Yellow.normal()),
            line_number: gutter_style,
        }
    }
}

I need to add tests before the PR, maybe today !

Nice! Would it be possible to render the icons after the linenumber line instead of before? like gitsigns.nvim?

Nice! Would it be possible to render the icons after the linenumber line instead of before? like gitsigns.nvim?

It's already rendered after the line number, do you mean outside of the statuscolumn ?

This is my nvim statuscolumn with gitsigns.nvim plugin, same rendering:

Image

And this is a bat screenshot using --style=numbers,changes option:

bat --style=numbers,changes /path/to/file

As you can see, same nvim rendering:

Image

I meant to say before the line numbers, like all the way on the left side. That's where gitsigns.nvim places them by default, right? At least, that's where mine are. Maybe it should be an option too?

I meant to say before the line numbers, like all the way on the left side. That's where gitsigns.nvim places them by default, right? At least, that's where mine are.

Gitsigns.nvim doesn't place it before or after, it just use the nvim default signs column, by default nvim has 3 columns(from left to right):

  • signs: used for both git signs and diagnostics
  • numbers: used for both relative and normal line numbers
  • folds: used for fold signs

Which means you can customize the statuscolumn and change the order or even add more columns, IMO, the default statuscolumn order isn't a good UX, for 3 main reasons:

  1. It's easier to notice the git sign when it's closer to the start line, i have tried both, much more better UX
  2. Moving from a modern editor like VSCode to nvim
  3. I don't like the idea of replacing the diagnostic with git sign(default nvim behavior)

That's why i'm using a 4 columns(not 3) with a custom statuscolumn, from left to right:

  • diagnostic signs(error, warning ...)
  • numbers
  • gitsigns
  • fold sign

Same statuscolumn used in VSCode, if you are interested, i can share my custom statuscolumn module used with nvim :)

Maybe it should be an option too?

Why not, I will push a draft PR first, then we can discuss all possible options ;)

Gitsigns.nvim doesn't place it before or after, it just use the nvim default signs column, by default nvim has 3 columns(from left to right)

Ah, alright thanks.

  1. It's easier to notice the git sign when it's closer to the start line, i have tried both, much more better UX

That's true. From a UX perspective, though, I think line numbers have more priority over git signs especially when relative line numbers are used. Looking at the line number relative to the line of code is done way more often. I don’t think the git signs need to stand out too much. That's why I prefer them positioned before the line numbers in nvim and would like the same behavior in Bat for consistency.

  1. Moving from a modern editor like VSCode to nvim

I don't think most users would be too thrown off by seeing the signs before the line number instead of after. I’ve moved on from VSCode and don’t really feel the need to stick to its design choices :)

  1. I don't like the idea of replacing the diagnostic with git sign(default nvim behavior)

Fair enough, i think this it also comes down to personal preference.

I hope the position can be made optional, but it’s up to the maintainer’s decision, i guess.

I hope the position can be made optional, but it’s up to the maintainer’s decision, i guess.

I haven't changed/touched the position, only changed the signs and added the read-from theme scopes if available.
I don't know if we can change the position, didn't check the code yet but if it's possible, i will add --statuscolumn flag
e.g. --statuscolumn=numbers,gitsigns or --statuscolumn=gitsigns,numbers, the flag name/value, it's up to the maintainer to choose/approve :)

Hi! I’m interested in working on this issue. Could you please assign it to me? Thank you!

Hi! I’m interested in working on this issue. Could you please assign it to me? Thank you!

hi @Raj-Dwivedi2005 , the feature already implemented, waiting for maintainer review, see PR #3407