Am 26.01.26 um 11:45 schrieb Chris Idema via GitGitGadget:
> From: Chris Idema <github_chris_idema@proton.me>
> 
> Tabs were not properly rendered in TK regardless of tab width settings.

Sorry, I cannot reproduce what I read into this sentence. When I change
the "Tab spacing" option in the Options dialog, the display changes to
the specified tab width. I'm using Tcl/Tk 8.6.

> Converting tab alignment to spaces before rendering in TK fixes this.

Do "Stage Line/Hunk for Commit" still work after this conversion?

-- Hannes

> Sorry, I cannot reproduce what I read into this sentence. When I change
the "Tab spacing" option in the Options dialog, the display changes to
the specified tab width. I'm using Tcl/Tk 8.6.

I use git for Windows version "2.52.0.windows.1" on Windows 11.

Here is how you can reproduce the problem:
mkdir test_tabs
cd test_tabs
git init
echo "" > test.c
git add .
git commit -m "initial commit"
echo -e "int test1\t= 5;\nint test11\t= 6;\nint test111\t= 6;\n" > test.c
git gui

> Do "Stage Line/Hunk for Commit" still work after this conversion?
I'm sorry but I don't know what this means.

-- Chris

-------- Original Message --------
On Monday, 01/26/26 at 13:15 Johannes Sixt <j6t@kdbg.org> wrote:
Am 26.01.26 um 11:45 schrieb Chris Idema via GitGitGadget:
> From: Chris Idema <github_chris_idema@proton.me>
>
> Tabs were not properly rendered in TK regardless of tab width settings.

Sorry, I cannot reproduce what I read into this sentence. When I change
the "Tab spacing" option in the Options dialog, the display changes to
the specified tab width. I'm using Tcl/Tk 8.6.

> Converting tab alignment to spaces before rendering in TK fixes this.

Do "Stage Line/Hunk for Commit" still work after this conversion?

-- Hannes

Am 26.01.26 um 14:32 schrieb GitHub Chris Idema:
> Here is how you can reproduce the problem:
> mkdir test_tabs
> cd test_tabs
> git init
> echo "" > test.c
> git add .
> git commit -m "initial commit"
> echo -e "int test1\t= 5;\nint test11\t= 6;\nint test111\t= 6;\n" > test.c
> git gui

So, you mean that if the tab width is set to 4, then the tab stops are
not aligned anymore?

>> Do "Stage Line/Hunk for Commit" still work after this conversion?
> I'm sorry but I don't know what this means.

These are commands in the context menu of the diff panel. They extract
the text from the widget and massage it into a patch. My suspicion is
that the patch text does not match the actual file contents, and so the
commands fail.

-- Hannes

> So, you mean that if the tab width is set to 4, then the tab stops are not aligned anymore?

Indeed. It's probably due to the + character shifting everything by 1 character.

> My suspicion is that the patch text does not match the actual file contents, and so the commands fail.

If you select and copy the text from the window with you mouse it won't match the patch. I didn't know people used it that way. I use it as a way to review my changes before staging.
I don't know if there is a way to make it that when you copy it will copy the original text and no the modified text.
If not then we should come up with a better way to align stops.

-- Chris

-------- Original Message --------
On Monday, 01/26/26 at 14:59 Johannes Sixt <j6t@kdbg.org> wrote:
Am 26.01.26 um 14:32 schrieb GitHub Chris Idema:
> Here is how you can reproduce the problem:
> mkdir test_tabs
> cd test_tabs
> git init
> echo "" > test.c
> git add .
> git commit -m "initial commit"
> echo -e "int test1\t= 5;\nint test11\t= 6;\nint test111\t= 6;\n" > test.c
> git gui

So, you mean that if the tab width is set to 4, then the tab stops are
not aligned anymore?

>> Do "Stage Line/Hunk for Commit" still work after this conversion?
> I'm sorry but I don't know what this means.

These are commands in the context menu of the diff panel. They extract
the text from the widget and massage it into a patch. My suspicion is
that the patch text does not match the actual file contents, and so the
commands fail.

-- Hannes

Am 26.01.26 um 15:43 schrieb GitHub Chris Idema:
>> So, you mean that if the tab width is set to 4, then the tab stops
>> are not aligned anymore?
> 
> Indeed. It's probably due to the + character shifting everything by 1 character.

BTW, this isn't a problem with a particular tab width. It happens with
the default width 8 as well.

>> My suspicion is that the patch text does not match the actual file
>> contents, and so the commands fail.
> 
> If you select and copy the text from the window with you mouse it
> won't match the patch. I didn't know people used it that way. I use
> it as a way to review my changes before staging.

I don't mean copy-and-paste. I mean the context menu commands. They stop
working (I suspect). This would be a show-stopper.

> I don't know if there is a way to make it that when you copy it will
> copy the original text and no the modified text. If not then we
> should come up with a better way to align stops.

I am not particularly fond of such a change. Years and years of reading
patch text has trained my brain to expect such misalignment to the
extent that even the absence of misalignment can sometimes indicate a
whitespace error.

-- Hannes

> I am not particularly fond of such a change. Years and years of reading
patch text has trained my brain to expect such misalignment to the
extent that even the absence of misalignment can sometimes indicate a
whitespace error.

The problem is not just incorrect alignment. It's also inconsistency. In gitk the alignment is correct. In the git gui window it's not.  The best solution would be to make the git gui window behave like gitk. I thought my change only affected the way it was displayed. I'm going to see if there is a better way.

-- Chris

-------- Original Message --------
On Monday, 01/26/26 at 15:52 Johannes Sixt <j6t@kdbg.org> wrote:
Am 26.01.26 um 15:43 schrieb GitHub Chris Idema:
>> So, you mean that if the tab width is set to 4, then the tab stops
>> are not aligned anymore?
>
> Indeed. It's probably due to the + character shifting everything by 1 character.

BTW, this isn't a problem with a particular tab width. It happens with
the default width 8 as well.

>> My suspicion is that the patch text does not match the actual file
>> contents, and so the commands fail.
>
> If you select and copy the text from the window with you mouse it
> won't match the patch. I didn't know people used it that way. I use
> it as a way to review my changes before staging.

I don't mean copy-and-paste. I mean the context menu commands. They stop
working (I suspect). This would be a show-stopper.

> I don't know if there is a way to make it that when you copy it will
> copy the original text and no the modified text. If not then we
> should come up with a better way to align stops.

I am not particularly fond of such a change. Years and years of reading
patch text has trained my brain to expect such misalignment to the
extent that even the absence of misalignment can sometimes indicate a
whitespace error.

-- Hannes

It appears inserting "apply_tab_size 1" fixes the issue. But I don't know if I'm inserting it in the right place.

-- Chris

"Chris Idema via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Chris Idema <github_chris_idema@proton.me>
>
> Signed-off-by: Chris Idema <github_chris_idema@proton.me>
> ---
>  git-gui/lib/diff.tcl | 24 +-----------------------
>  1 file changed, 1 insertion(+), 23 deletions(-)
>
> diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
> index 2e13f8c776..0f0951cc57 100644
> --- a/git-gui/lib/diff.tcl
> +++ b/git-gui/lib/diff.tcl
> @@ -12,27 +12,6 @@ proc apply_tab_size {{firsttab {}}} {
>  	}
>  }
>  
> -proc expand_tabs {line {startcol -1}} {
> -	# startcol set to -1, because in preview the lines start with a '+', '-', or ' '
> -	global repo_config
> -
> -	set col $startcol
> -	set out ""
> -
> -	foreach char [split $line ""] {
> -		if {$char eq "\t"} {
> -			set spaces [expr {$repo_config(gui.tabsize) - ($col % $repo_config(gui.tabsize))}]
> -			append out [string repeat " " $spaces]
> -			incr col $spaces
> -		} else {
> -			append out $char
> -			incr col
> -		}
> -	}
> -
> -	return $out
> -}
> -
>  proc clear_diff {} {
>  	global ui_diff current_diff_path current_diff_header
>  	global ui_index ui_workdir
> @@ -516,9 +495,8 @@ proc read_diff {fd conflict_size cont_info} {
>  			}
>  		}
>  		set mark [$ui_diff index "end - 1 line linestart"]
> -		set line [expand_tabs $line]
> +		apply_tab_size 1
>  		$ui_diff insert end "$line" $tags
> -

Why does this series first add proc expand_tabs, only to remove its
use in this second step?  Shouldn't these two patches be squashed
into one, and explain why we want to use "apply_tab_size 1" here?

It smells fishy to do "apply_tab_size 1" here in "proc clear_diff".

It is called from "proc show_diff" but the latter, after it calls
clear_diff, calls "apply_tab_size 0".  Doesn't that defeat the
effect of this new call added to "proc clear_diff"?

By the way, this has nothing to do with your change, but the only
existing use of "apply_tab_size 1" is also somewhat curious.  When
"proc read_diff" detects that a patch hunk header has three (not the
usual two) at-signs, it calls "apply_tab_size 1", presumably to
adjust to the fact that combined diff has two leading columns used
to signal added/removed/context lines, instead of one.

Apparently the author of the original code thought that it is a good
idea for such a payload if first tab moves 1 column, and second and
subsequent tabs taking gui.tabsize after that tabstop.  A line in
combined diff uses two leading columns for line prefix.  Isn't it
curious that these two patches under discussion claim that the same
exact setting of "apply_tab_size 1" is appropriate for _anything_
that is shown in the $ui_diff widget prepared with "proc
clear_diff"?

Presumably most of the time, the output format would use just a
single leading column for line prefix added (+), removed (-), or
context ( ).

Both cannot be correct at the same time, can they?

So, either the original author is wrong and the current code is
broken with or without your change when it shows a combined diff, or
these patches is wrong and there is off-by-one bug somwhere.

Puzzled and curious ...

Junio C Hamano <gitster@pobox.com> writes:

> By the way, this has nothing to do with your change, but the only
> existing use of "apply_tab_size 1" is also somewhat curious.  When
> "proc read_diff" detects that a patch hunk header has three (not the
> usual two) at-signs, it calls "apply_tab_size 1", presumably to
> adjust to the fact that combined diff has two leading columns used
> to signal added/removed/context lines, instead of one.
>
> Apparently the author of the original code thought that it is a good
> idea for such a payload if first tab moves 1 column, and second and
> subsequent tabs taking gui.tabsize after that tabstop.  A line in
> combined diff uses two leading columns for line prefix.  Isn't it
> curious that these two patches under discussion claim that the same
> exact setting of "apply_tab_size 1" is appropriate for _anything_
> that is shown in the $ui_diff widget prepared with "proc
> clear_diff"?
>
> Presumably most of the time, the output format would use just a
> single leading column for line prefix added (+), removed (-), or
> context ( ).
>
> Both cannot be correct at the same time, can they?
>
> So, either the original author is wrong and the current code is
> broken with or without your change when it shows a combined diff, or
> these patches is wrong and there is off-by-one bug somwhere.
>
> Puzzled and curious ...

Apparently the whole thing comes from a43c5f51 (git-gui: add
configurable tab size to the diff view, 2012-02-12).

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 do not know if those involved in the original commit are around,
but just in case if they remember, I'll CC them.


commit a43c5f51a4b1e56b746295f19daa240283092005
Author: Michael Lutz <michi@icosahedron.de>
Date:   Sun Feb 12 16:55:17 2012 +0100

    git-gui: add configurable tab size to the diff view
    
    For Tk 8.5 the "wordprocessor" mode allows us to get a bit fancy for merge
    diffs and intend the tabs by one to compensate for the additional diff
    marker at the line start.
    
    The code is heavily based on how gitk handles tabs.
    
    Signed-off-by: Michael Lutz <michi@icosahedron.de>
    Signed-off-by: Pat Thoyts <patthoyts@users.sourceforge.net>

diff --git a/git-gui.sh b/git-gui.sh
index 6cbb36eab6..bf68699616 100755
--- a/git-gui.sh
+++ b/git-gui.sh
@@ -912,6 +912,7 @@ set default_config(gui.fontdiff) [font configure font_diff]
 set default_config(gui.maxfilesdisplayed) 5000
 set default_config(gui.usettk) 1
 set default_config(gui.warndetachedcommit) 1
+set default_config(gui.tabsize) 8
 set font_descs {
 	{fontui   font_ui   {mc "Main Font"}}
 	{fontdiff font_diff {mc "Diff/Console Font"}}
diff --git a/lib/diff.tcl b/lib/diff.tcl
index b0a5180af7..0d56986215 100644
--- a/lib/diff.tcl
+++ b/lib/diff.tcl
@@ -1,6 +1,19 @@
 # git-gui diff viewer
 # Copyright (C) 2006, 2007 Shawn Pearce
 
+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}]]
+	} elseif {$have_tk85 || $repo_config(gui.tabsize) != 8} {
+		$ui_diff configure -tabs [expr {$repo_config(gui.tabsize) * $w}]
+	} else {
+		$ui_diff configure -tabs {}
+	}
+}
+
 proc clear_diff {} {
 	global ui_diff current_diff_path current_diff_header
 	global ui_index ui_workdir
@@ -105,6 +118,8 @@ proc show_diff {path w {lno {}} {scroll_pos {}} {callback {}}} {
 
 	set cont_info [list $scroll_pos $callback]
 
+	apply_tab_size 0
+
 	if {[string first {U} $m] >= 0} {
 		merge_load_stages $path [list show_unmerged_diff $cont_info]
 	} elseif {$m eq {_O}} {
@@ -401,7 +416,10 @@ proc read_diff {fd conflict_size cont_info} {
 
 		# -- Automatically detect if this is a 3 way diff.
 		#
-		if {[string match {@@@ *} $line]} {set is_3way_diff 1}
+		if {[string match {@@@ *} $line]} {
+			set is_3way_diff 1
+			apply_tab_size 1
+		}
 
 		if {$::current_diff_inheader} {
 
diff --git a/lib/option.tcl b/lib/option.tcl
index 23c9ae72a4..b5b6b2fea6 100644
--- a/lib/option.tcl
+++ b/lib/option.tcl
@@ -161,6 +161,7 @@ proc do_options {} {
 		{b gui.warndetachedcommit {mc "Warn before committing to a detached head"}}
 		{s gui.stageuntracked {mc "Staging of untracked files"} {list "yes" "no" "ask"}}
 		{b gui.displayuntracked {mc "Show untracked files"}}
+		{i-1..99 gui.tabsize {mc "Tab spacing"}}
 		} {
 		set type [lindex $option 0]
 		set name [lindex $option 1]

> Why does this series first add proc expand_tabs, only to remove its
use in this second step?  Shouldn't these two patches be squashed
into one, and explain why we want to use "apply_tab_size 1" here?

Because I received feedback on the first commit and realized the 
second solution is better. I didn't know you could squash the patches
ones the first one was reviewed. 
I generally don't like rewriting history, but I will be squashing the 
commits.
For some reason Johannes Sixt disappeared from the mailing list. 
I've never used such a clunky interface before. 
I guess linux users like self-flagellation.

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

The bug has been there for 14 years I guess. In gitk it works as 
expected.In git diff it works as expected when setting up the pager.
In git-gui it doesn't. The alignment is inconsistent with gitk.
For code review it's horrible.
Here is a link to 2 images that show the before and after:
https://github.com/git/git/pull/2179#issuecomment-3799576864

-- Chris

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

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

> 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

Am 28.01.26 um 15:02 schrieb GitHub Chris Idema:
>> 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.
> ...
> Can you show me how to get content without patch text in the window?

Sorry, I meant "untracked file". When the text of an untracked file is
displayed, we do not want to offset the tabstops.

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

This was not a suggested fix, but a citation of the patch that
introduced the function. The variable has since been eliminated.

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

It may fix the problem for regular patch text. But I doubt that it is a
correct fix for combined-diff text, because that needs offset 2.

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

Earlier, I said that I'm not fond of such a change. But I changed my
mind. I hadn't noticed so far that Gitk applies customized tabstops. Git
GUI and Gitk need not emulate the behavor of terminal windows faithfully
and can be more clever as far as tabstops are concerned.

-- Hannes

Am 28.01.26 um 11:20 schrieb Chris Idema via GitGitGadget:
> From: Chris Idema <github_chris_idema@proton.me>
> 
> Tab stop width was not properly rendered in TK regardless of
> tab width setting. The + or minus character at start of line made
> tabs align incorrectly.

This is a patch for Git GUI. Please use the subject prefix "git-gui:".
The file name need not be mentioned.

Please have a look at existing commits and mimic the style of the commit
subject and body text. In particular:

- Use present tense to describe the current state. Elaborate what the
problem is. Assume that readers haven't looked at the code for some time
and guide them to the problem point (i.e., provide some context).

- Use imperative mood to describe the change as if you instruct someone
to make the change.

I suggest this subject:

git-gui: shift tabstops to account for the first column of context diffs

> 
> Signed-off-by: Chris Idema <github_chris_idema@proton.me>
> ---
>     diff.tcl: made alignment of tabs in git-gui diff consistent with gitk
>     
>     cc: Johannes Sixt j6t@kdbg.org

Just FYI, this message didn't arrive in my mailbox despite this line.

> diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
> index 442737ba4f..7da6e5ccae 100644
> --- a/git-gui/lib/diff.tcl
> +++ b/git-gui/lib/diff.tcl
> @@ -495,6 +495,7 @@ proc read_diff {fd conflict_size cont_info} {
>  			}
>  		}
>  		set mark [$ui_diff index "end - 1 line linestart"]
> +		apply_tab_size 1
>  		$ui_diff insert end $line $tags
>  		if {[string index $line end] eq "\r"} {
>  			$ui_diff tag add d_cr {end - 2c}

If you look at commit a43c5f51a4b1, you will notice that it intended to
apply "magic" tabstops only to 3-way-diffs. It did not intend to "fix"
regular patch text. Without the change, 3-way-diffs would become even
more misaligned, because these have two initial positions instead of
just one. To fix the additional misalignment, it applies the offset 1 to
the tabstops. But this does not fix the original misalignment.

You now want to fix the original misalignment. Therefore, you have to
apply the offset 1 for regular patch text, but offset 2 to 3-way-diffs.
And, in addition, no offset if file contents are displayed.

-- Hannes

> This is a patch for Git GUI. Please use the subject prefix "git-gui:".
The file name need not be mentioned.

Thank you.

>If you look at commit a43c5f51a4b1, you will notice that it intended to
apply "magic" tabstops only to 3-way-diffs

I see it now.
I was able to test:
- "Modified, not staged", needs "apply_tab_size 1"
- "Staged for commit", needs "apply_tab_size 1"
- "Requires merge resolution", doesn't work and needs "apply_tab_size 2"
- "Untracked, not staged", handled somewhere else, works
- "Missing", needs "apply_tab_size 1"
- "Staged for removal", needs "apply_tab_size 1"

So I need to make some changes.

-- Chris

Johannes Sixt <j6t@kdbg.org> writes:

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

> Earlier, I said that I'm not fond of such a change. But I changed my
> mind. I hadn't noticed so far that Gitk applies customized tabstops. Git
> GUI and Gitk need not emulate the behavor of terminal windows faithfully
> and can be more clever as far as tabstops are concerned.

I just peeked what gitk does, and it does use "settabs 0" (the
equivalent of "apply_tab_size 0" in gitk world) for plain files,
"settabs 1" for one parent regular commits, and "settabs $np" for
n-parent merges, so what Chris is doing here makes git-gui match
what gitk has been doing since 32f1b3e4 (gitk: Fix the tab setting
in the diff display window, 2007-09-28) for close to 20 years ;-).

Having said that, the fact that they have been allowed to be
different for so long tells me that the way characters immediately
after tabs have been displayed in git-gui bothered nobody for a long
time, and calling it a "show stopper" and "unreadable" is a great
exaggeration, I must say.

Thanks.

"Chris Idema via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Chris Idema <github_chris_idema@proton.me>
>
> Tab stop width was not properly rendered in TK regardless of
> tab width setting. The + or minus character at start of line made
> tabs align incorrectly.

If git-gui has a feature to show file contents, not comparison of
the old and the new versions of a file as a diff, there won't be
plus or minus at the beginning.  The above paragraph of course is
mostly OK, but it would be more helpful to qualify it by talking
about "diff" somewhere.

    Because the diff view spends the leftmost column for plus sign
    for added line, minus sign for removed line, etc., a tab that
    would push the next character to multiple of 8 (or gui.tabsize)
    column may appear narrower by 1 column.

    Compensate for this by setting tabstops at 9th, 17th, 25th,
    ... columns (or 1+multiple of gui.tabsize) for showing a single
    parent diff, and shift by 2 columns for showing a two parent
    diff.

or something.

I noticed that gitk has code to deal with octopus merges (i.e., a
merge does not necessarily have two parents, but it is possible to
have more parents), but git-gui assumes that merges with two parents
are the only ones that are worth caring about.  Correcting for this
may be almost trivial, but I do not think it falls into the scope of
this topic.  But at least I think this topic should adjust existing
"apply_tab_size 1" used for two-parent merge combined diff to use 2.

>  git-gui/lib/diff.tcl | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
> index 442737ba4f..7da6e5ccae 100644
> --- a/git-gui/lib/diff.tcl
> +++ b/git-gui/lib/diff.tcl
> @@ -495,6 +495,7 @@ proc read_diff {fd conflict_size cont_info} {
>  			}
>  		}
>  		set mark [$ui_diff index "end - 1 line linestart"]
> +		apply_tab_size 1
>  		$ui_diff insert end $line $tags
>  		if {[string index $line end] eq "\r"} {
>  			$ui_diff tag add d_cr {end - 2c}
>
> base-commit: 1faf5b085a171f9ba9a6d7a446e0de16acccb1dc

Johannes Sixt <j6t@kdbg.org> writes:

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

True.  So instead of setting tabstops at 1, 9, 17, 25, ..., gitk
does 9, 17, 25, 33, ..., which makes more sense, but there is no
practical difference, no?  Because the first column will be the
plus, minus, or space and it will never be a tab.

> From: Junio C Hamano <gitster@pobox.com>
>
> calling it a "show stopper" and "unreadable" is a great
> exaggeration, I must say.

We use clang-format to format most of our code.
But we don't have that always available.
So it's good to review indentation changes prior to commit.
And we use either git diff or Git Gui for that.
For many file changes I prefer Git Gui as you don't need to scroll.

For git diff there is a way to configure tab size to 4:
git config --global core.pager 'less -x1,5'
source: https://stackoverflow.com/a/10584237/15307950

For Git Gui and Gitk there is also a tab setting.
But only in Git Gui it didn't work as expected.
So with show stopper I meant that it's the only odd one.
And since the code already uses apply_tab_size it makes sense to just 
apply it correctly in all scenarios.

My latest commit was tested for:
- "Modified, not staged"
- "Staged for commit"
- "Requires merge resolution"
- "Untracked, not staged"
- "Missing"
- "Staged for removal"

And it worked on my side. 
@@@ needs apply_tab_size 2
@@ needs apply_tab_size 1
the rest was already handled correctly

> From: Junio C Hamano <gitster@pobox.com>
>
> I noticed that gitk has code to deal with octopus merges

I would love to know how such a merge can be replicated.
Is it also possible to have such a merge visible in Git Gui?

-- Chris

Am 29.01.26 um 09:31 schrieb GitHub Chris Idema:
>> From: Junio C Hamano <gitster@pobox.com>
>> I noticed that gitk has code to deal with octopus merges
> 
> I would love to know how such a merge can be replicated.
> Is it also possible to have such a merge visible in Git Gui?
This case is not relevant for Git GUI, because it can only show what is
in the index. We have only "theirs" and "ours", and together with the
current file contents that's a 3-way diff.

-- Hannes

Johannes Sixt <j6t@kdbg.org> writes:

> Am 29.01.26 um 09:31 schrieb GitHub Chris Idema:
>>> From: Junio C Hamano <gitster@pobox.com>
>>> I noticed that gitk has code to deal with octopus merges
>> 
>> I would love to know how such a merge can be replicated.
>> Is it also possible to have such a merge visible in Git Gui?
> This case is not relevant for Git GUI, because it can only show what is
> in the index. We have only "theirs" and "ours", and together with the
> current file contents that's a 3-way diff.

OK.  If it does not show existing merge commits, then I agree that
only 3-way is relevant.  Thanks for a clarification.

Am 29.01.26 um 12:09 schrieb Chris Idema via GitGitGadget:
> From: Chris Idema <github_chris_idema@proton.me>
> 
> When reviewing a file before staging you want its content aligned using
> gui.tabsize. The prefixing of lines with +, - or space characters should
> not change this alignment. In gitk this is done correctly. In Git Gui not.
> 
> Signed-off-by: Chris Idema <github_chris_idema@proton.me>
> ---

> 
>  git-gui/lib/diff.tcl | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
> index 442737ba4f..8be1a613fb 100644
> --- a/git-gui/lib/diff.tcl
> +++ b/git-gui/lib/diff.tcl
> @@ -385,6 +385,8 @@ proc read_diff {fd conflict_size cont_info} {
>  		#
>  		if {[string match {@@@ *} $line]} {
>  			set is_3way_diff 1
> +			apply_tab_size 2
> +		} elseif {[string match {@@ *} $line]} {
>  			apply_tab_size 1
>  		}
>  
Just "else" without a condition would have been sufficient, but we can
do it this way as well.

I've rewritten the commit message like so:

    git-gui: shift tabstops to account for the first column of patch text
    
    When reviewing a change before staging, it is desirable to see text after
    tabstops aligned the same way as in the text editor. However, since there
    is always an additional character in column one in patch text ('+', '-',
    or space), the alignment is broken if text before the first tab character
    is just long enough to push the stop to the next tab position.
    
    Commit a43c5f51a4b1 (git-gui: add configurable tab size to the diff view,
    2012-02-12) added infrastructure that manipulates the tabstop positions
    of the Tk text widget. However, it does so only when a 3-way diff is
    shown and only so that it takes into account the one additional markup at
    the beginning of lines. This only achieved that alignment does not get
    worse for 3-way diffs compared to regular patch text, but left misaligned
    text in regular patch text unmodified.
    
    Use and modify this infrastructure to shift tabstops by one position for
    regular patch text and two positions for 3-way diffs. Existing code
    already resets the tabstops to an unshifted position when contents of
    untracked files are displayed.
    
    Signed-off-by: Chris Idema <github_chris_idema@proton.me>
    [j6t: extend commit message]
    Signed-off-by: Johannes Sixt <j6t@kdbg.org>

In particular there was no bug; this is a new feature.

Thanks,
-- Hannes