From: Johannes Sixt <j6t@kdbg.org>
Date: Wed, 28 Jan 2026 13:40:02 GMT
Subject: Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces.
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:
> 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