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

Fix diffFilter when zero lines are styled #1916

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

th1000s
Copy link
Collaborator

@th1000s th1000s commented Nov 26, 2024

With interactive.diffFilter = delta --color-only delta is called by e.g. git add -p, but in this mode git hides the terminal from the pager.

Plus/minus lines correctly use ANSI sequences to paint up to to the end of the line, but zero lines always used spaces. This needs the terminal width, but it is not available for diffFilter. So the fallback of 80 is used, and zero styles did not extend to the full terminal width.

Since zero lines are only rarely styled (e.g. via
--zero-style='syntax "#1d1f21" dim'), this was never noticed.

This also crashed delta when a zero line was longer than 80.

With `interactive.diffFilter = delta --color-only` delta is called
by e.g. `git add -p`, but in this mode git hides the terminal from 
the pager.

Plus/minus lines correctly use ANSI sequences to paint up to to the
end of the line, but zero lines always use spaces. This needs the terminal
width, but it is not available for diffFilter. So the fallback
of 80 is used, and zero styles did not extend to the full terminal width.

Since zero lines are only rarely styled (e.g. via
`--zero-style='syntax "#1d1f21" dim'`), this was never noticed.

This also crashed delta when a zero line was longer than 80.
@th1000s th1000s requested a review from dandavison November 26, 2024 21:42
@th1000s
Copy link
Collaborator Author

th1000s commented Nov 26, 2024

This unfortunately breaks a bunch of unrelated tests, any idea where the #, e.g. commit 94907c to #commit 94907c, on the first line comes from?

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

Successfully merging this pull request may close these issues.

1 participant