dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
21.82k stars 364 forks source link

Delta appending ^[[OK at the end of each modified line ( color related i think ) #1490

Closed ippsav closed 1 year ago

ippsav commented 1 year ago

20230727_09h31m05s_grim

Delta version: delta 0.16.5

dandavison commented 1 year ago

Hi @ippsav, this is likely to be a problem with your environment (terminal emulator) since it hasn't been reported by other users.

Please try a different terminal emulator and see if you see the same problem.

ippsav commented 1 year ago

Terminal: alacritty Session manager: tmux OS: Arch linux

Will try !

sophiabrandt commented 1 year ago

I'm encountering the same issue when I use the fish shell.

It seems to make no difference if I use tmux or not.

Emulator: kitty OS: macOs M1

It works when I use zsh with iterm2. It also works with zsh and kitty.

I've been searching through the fish shell github, but haven't found an obvious solution.

Are you using fish shell as well @ippsav ?

dandavison commented 1 year ago

Hi @sophiabrandt, @ippsav. Hm, you probably already know this but for the record that code is ANSI CSI code "Erase in Line" (https://en.wikipedia.org/wiki/ANSI_escape_code). Delta uses it to cause the background colour to extend to the right-hand edge of the terminal window.

FWIW I can't reproduce it with fish 3.6.1, Alacritty, MacOS M2, with or without tmux.

ippsav commented 1 year ago

@sophiabrandt i am using zsh, it's weird i think it's related to terminal settings, or tmux settings with color stuff

sophiabrandt commented 1 year ago

@dandavison in any case, many thanks for your work on delta. ❤️

michenriksen commented 1 year ago

@ippsav @sophiabrandt do you use most or maybe some other fancy pager? (echo $PAGER)

I saw the same strange rendering problem, and I finally narrowed it down to export PAGER='most' in my .zshrc file.

I fixed it by configuring delta to use less in my .gitconfig:

[delta]
  pager = less

@dandavison I found this in the README for bat:

Note: If PAGER is more or most, bat will silently use less instead to ensure support for colors.

Maybe it would make sense for delta to do the same?

sophiabrandt commented 1 year ago

@michenriksen yes! That's it! Thank you. Never would have thought of the pager... 😅

ippsav commented 1 year ago

@michenriksen Worked for me as well thanks man !

dandavison commented 1 year ago

Excellent, thanks @michenriksen! Yeah I'm not quite sure where to slot in this advice; I don't think the README instructions should recommend

[delta]
    pager = less

Since IIRC that overrides less config the user might have e.g. in the LESS env var.

I think that the best solution for this is @clnoll's delta --doctor command: https://github.com/dandavison/delta/pull/1193. That will alert the user if they are not using less.

michenriksen commented 1 year ago

@dandavison yeah, adding pager = less in the README instructions would probably not be the best, that's why I wondered if it makes sense for delta to do what bat does:

Note: If PAGER is more or most, bat will silently use less instead to ensure support for colors.

dandavison commented 1 year ago

@michenriksen Right, thanks I think that's a great idea. Researching briefly, it looks like if we change the entry for the bat crate in Cargo.toml to

bat = { version = "0.23.0", default-features = false, features = ["minimal-application", "paging", "regex-onig"] }

Then the bat function get_pager_executable is available to us. I wonder if we can modify the delta code that invokes the pager to inherit the logic you suggest from bat, without copying more code from bat.

Backstory: when I first wrote delta, bat didn't exist as a library, and I copied/vendored some bat code dealing with invoking the pager (delta also uses bat's awesome collection of syntax highlighting assets of course). Subsequently, bat became available as a library, and as they make things available in the library, delta is trying to use them from the library. Delta's copied/vendored bat code is in src/utils/bat/.

If anyone would like to make the change I'm suggesting here that would be fantastic.

dandavison commented 1 year ago

[Closed since this issue is solved, but very happy for discussion to continue.]

ippsav commented 1 year ago

@dandavison that sounds like something nice to try to do, i ll give it a try !

dandavison commented 1 year ago

@ippsav excellent, thanks!

ippsav commented 1 year ago

@dandavison I ve been reading the code trying to get the hang of it, i ll be making a pr soon i ll keep you updated it seems to be solved !

ippsav commented 1 year ago

@dandavison ready for review keep me updated !

michenriksen commented 1 year ago

Nice one, @ippsav! 👏

ippsav commented 1 year ago

@michenriksen thanks man