Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces.

From: GitHub Chris Idema <github_chris_idema@proton.me>
Date: Jan 28, 2026, 14:02 UTC
Message-ID: <Rrnh0ugGQ5ef_s-3W0Tive8HA9R0_9Cq6yK7K6SS6Jr3kPigHai3jzxvphTRNXXNhCnor2TMV8UjKEi5U27VOLBf-R4g6VbJBLq8PQH9kCI=@proton.me>
In-Reply-To: <71494127-c17d-4fd9-a69d-1f547205ac8f@kdbg.org>

>I concur. Also, "apply_tab_size 0" is needed when the contents of an

unstaged file are shown instead of patch text.

Can you explain why it's needed? The file in my example is unstaged and it's a patch text. So these are not mutually exclusive. Even for a staged file the context lines are indented by 1 space instead of a + or - character. So tab stop width is also incorrect for context lines. Can you show me how to get content without patch text in the window?

> +	if {$have_tk85 && $firsttab != 0} {

Gives me the error "can't read "have_tk85": no such variable" If I substitute 1 or 0 for have_tk85 it doesn't fix the alignment.

I'm open for suggestions. My 1 line code change fixes the problem, but if it is not the official way to do it or if it introduces other problems feel free to suggest another fix.

For reference here are the screenshots of the problem: https://github.com/git/git/pull/2179#issuecomment-3799576864

For us this bug is a show stopper that makes the diff in the git-gui window by default unreadable.

-- Chris

On Wednesday, January 28th, 2026 at 14:40, Johannes Sixt <j6t@kdbg.org> wrote:

Show 36 quoted lines

> Am 28.01.26 um 00:26 schrieb Junio C Hamano:
> 
> > It is clear that "apply_tab_size 0" is designed for a single-parent
> > diff, while "apply_tab_size 1" is designed for two parents diff. If
> > this new series to make sense, I think it should argue why that
> > setting that users are already familiar with for the past 14 years
> > is wrong, and "apply_tab_size 1" is more appropriate for a single
> > parent diff (and presumably "apply_tab_size 2" is better for two
> > aprent diff), I think.
> 
> 
> I concur. Also, "apply_tab_size 0" is needed when the contents of an
> unstaged file are shown instead of patch text.
> 
> > +proc apply_tab_size {{firsttab {}}} {
> > + global have_tk85 repo_config ui_diff
> > +
> > + set w [font measure font_diff "0"]
> > + if {$have_tk85 && $firsttab != 0} {
> > + $ui_diff configure -tabs [list [expr {$firsttab * $w}] [expr {($firsttab + $repo_config(gui.tabsize)) * $w}]]
> 
> 
> I think that these values for tabstops aren't optimal. It does not make
> sense to have tabstop at column 1 for diff output, because there is
> always at least one character ('+', '-', or SP), so that the first tab
> would jump right to the second stop. In Gitk, the initial version looked
> like this as well, but it this was changed soon after.
> 
> > + } elseif {$have_tk85 || $repo_config(gui.tabsize) != 8} {
> > + $ui_diff configure -tabs [expr {$repo_config(gui.tabsize) * $w}]
> > + } else {
> > + $ui_diff configure -tabs {}
> > + }
> > +}
> 
> -- Hannes