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

From: Johannes Sixt <j6t@kdbg.org>
Date: Jan 28, 2026, 13:40 UTC
Message-ID: <71494127-c17d-4fd9-a69d-1f547205ac8f@kdbg.org>
In-Reply-To: <xmqqfr7qeixq.fsf@gitster.g>

Am 28.01.26 um 00:26 schrieb Junio C Hamano:

Show 7 quoted lines

> 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.

Show 6 quoted lines

> +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.

Show 6 quoted lines

> +	} elseif {$have_tk85 || $repo_config(gui.tabsize) != 8} {
> +		$ui_diff configure -tabs [expr {$repo_config(gui.tabsize) * $w}]
> +	} else {
> +		$ui_diff configure -tabs {}
> +	}
> +}

-- Hannes