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.

Sign up for GitHub

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

Jump to bottom

Broken line breaks when using it with delta as git-diff pager #2307

Closed
arctic-penguin opened this issue Sep 8, 2022 · 8 comments · Fixed by #2308
Closed

Broken line breaks when using it with delta as git-diff pager #2307

arctic-penguin opened this issue Sep 8, 2022 · 8 comments · Fixed by #2308
Labels
bug Something isn't working

Comments

@arctic-penguin
Copy link

arctic-penguin commented Sep 8, 2022 •
edited
Loading

What steps will reproduce the bug?

  1. export PAGER=bat
  2. git config pager.diff=delta
  3. Go to any git clone that contains a long line, remove it.

What happens?
Line breaks are broken.
neu

...

What did you expect to happen instead?
Line breaks are okay.
alt

...

How did you install bat?

pacman -S bat, can also reproduce it via cargo install, as well as git clone followed by cargo install --path .

  • Good version: bat 0.21.0 (405e5f74)
  • Bad version: bat 0.22.0 (1f9519d8) (release)
  • Bad version: bat 0.22.0 (6680f65) (HEAD)
  • git bisect points to 899fdbb

The error is triggered when running git-diff, with pager.diff set to delta. My $PAGER is set to bat. Therefore I don't know how to apply the --diagnostics part.

P.S.: The gdi command is an alias to git diff.
@arctic-penguin arctic-penguin added the bug Something isn't working label Sep 8, 2022
@arctic-penguin
Copy link
Author

I just found it affects more applications. This is happens when installing updates on Arch Linux with the AUR-helper paru.
screen

@Enselic
Copy link
Collaborator

Enselic commented Sep 8, 2022

Quick question before I have time for a more deeper investigation: Does it look OK to you if you disable wrapping? Like this:

export PAGER='bat --wrap=never'

@arctic-penguin
Copy link
Author

Yes, it does. What does that mean for me?

@Enselic
Copy link
Collaborator

Enselic commented Sep 9, 2022

@sharkdp I think we should revert #2189 and do a 0.22.1 point release ASAP. Do you agree?

I didn't know about this use case, but it seems like a basic use case, so the short term solution should be to revert that PR, I think. That will make #2185 occur again, but that seems like more of an edge case than the use case in this issue.

Enselic added a commit to Enselic/bat that referenced this issue Sep 9, 2022
@Enselic
Revert "Remove code that tries to handle ANSI escape inputs"
149140b 
This reverts commit 8174e02. Turns out
it is needed for a common use case, see
sharkdp#2307.

It is not a clean revert, because I adjust CHANGELOG.md and also add a
comment to the test. I also had to resolve a small `use` conflict.
@Enselic Enselic mentioned this issue Sep 9, 2022
Merged
15 tasks
@Enselic
Copy link
Collaborator

Enselic commented Sep 9, 2022 •
edited
Loading

PR that does the revert and also prepares for a v0.22.1 release: #2308

@sharkdp
Copy link
Owner

sharkdp commented Sep 9, 2022

@sharkdp I think we should revert #2189 and do a 0.22.1 point release ASAP. Do you agree?

Yes, thanks.

Enselic added a commit that referenced this issue Sep 10, 2022
@Enselic
Revert "Remove code that tries to handle ANSI escape inputs"
352309b 
This reverts commit 8174e02. Turns out
it is needed for a common use case, see
#2307.

It is not a clean revert, because I adjust CHANGELOG.md and also add a
comment to the test. I also had to resolve a small `use` conflict.
@Enselic
Copy link
Collaborator

Enselic commented Sep 10, 2022 •
edited
Loading

Fix released as v0.22.1.

@arctic-penguin
Copy link
Author

Thank you for the very quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Revert "Remove code that tries to handle ANSI escape inputs", prepare for v0.22.1 Enselic/bat
3 participants
@Enselic @sharkdp @arctic-penguin