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

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

Closed
ippsav opened this issue Jul 27, 2023 · 19 comments
Closed

Comments

@ippsav
Copy link
Contributor

ippsav commented Jul 27, 2023

20230727_09h31m05s_grim

Delta version: delta 0.16.5

@dandavison
Copy link
Owner

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

  • Which terminal emulator are you using?
  • Are you using tmux or screen?
  • What OS are you using?

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

@ippsav
Copy link
Contributor Author

ippsav commented Jul 27, 2023

Terminal: alacritty
Session manager: tmux
OS: Arch linux

Will try !

@sophiabrandt
Copy link

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
Copy link
Owner

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
Copy link
Contributor Author

ippsav commented Jul 27, 2023

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

@sophiabrandt
Copy link

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

@michenriksen
Copy link

@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
Copy link

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

@ippsav
Copy link
Contributor Author

ippsav commented Jul 29, 2023

@michenriksen Worked for me as well thanks man !

@dandavison
Copy link
Owner

dandavison commented Jul 29, 2023

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: #1193. That will alert the user if they are not using less.

@michenriksen
Copy link

@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
Copy link
Owner

@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
Copy link
Owner

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

@ippsav
Copy link
Contributor Author

ippsav commented Jul 29, 2023

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

@dandavison
Copy link
Owner

@ippsav excellent, thanks!

@ippsav
Copy link
Contributor Author

ippsav commented Jul 29, 2023

@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
Copy link
Contributor Author

ippsav commented Jul 29, 2023

@dandavison ready for review keep me updated !

@michenriksen
Copy link

Nice one, @ippsav! 👏

@ippsav
Copy link
Contributor Author

ippsav commented Jul 30, 2023

@michenriksen thanks man

haplo added a commit to haplo/dotfiles that referenced this issue Mar 14, 2024
Turns out this was because of using most as default pager.

See dandavison/delta#1490 for full discussion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants