* [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces
@ 2026-01-26 10:45 Chris Idema via GitGitGadget
2026-01-26 12:15 ` Johannes Sixt
2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget
0 siblings, 2 replies; 30+ messages in thread
From: Chris Idema via GitGitGadget @ 2026-01-26 10:45 UTC (permalink / raw)
To: git; +Cc: Chris Idema, Chris Idema
From: Chris Idema <github_chris_idema@proton.me>
Tabs were not properly rendered in TK regardless of tab width settings.
Converting tab alignment to spaces before rendering in TK fixes this.
Does not fix alignment issues in gitk.
Signed-off-by: Chris Idema <github_chris_idema@proton.me>
---
diff.tcl: Fixed alignment of tabs in git-gui diff by using spaces.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2179%2FChrisIdema%2Ffix-gitgui-diff-tab-alignment-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2179/ChrisIdema/fix-gitgui-diff-tab-alignment-v1
Pull-Request: https://github.com/git/git/pull/2179
git-gui/lib/diff.tcl | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl
index 442737ba4f..2e13f8c776 100644
--- a/git-gui/lib/diff.tcl
+++ b/git-gui/lib/diff.tcl
@@ -12,6 +12,27 @@ 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
@@ -495,7 +516,9 @@ proc read_diff {fd conflict_size cont_info} {
}
}
set mark [$ui_diff index "end - 1 line linestart"]
- $ui_diff insert end $line $tags
+ set line [expand_tabs $line]
+ $ui_diff insert end "$line" $tags
+
if {[string index $line end] eq "\r"} {
$ui_diff tag add d_cr {end - 2c}
}
base-commit: 1faf5b085a171f9ba9a6d7a446e0de16acccb1dc
--
gitgitgadget
^ permalink raw reply related [flat|nested] 30+ messages in thread* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 10:45 [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces Chris Idema via GitGitGadget @ 2026-01-26 12:15 ` Johannes Sixt 2026-01-26 13:32 ` GitHub Chris Idema 2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget 1 sibling, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-26 12:15 UTC (permalink / raw) To: Chris Idema; +Cc: git, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 12:15 ` Johannes Sixt @ 2026-01-26 13:32 ` GitHub Chris Idema 2026-01-26 13:59 ` Johannes Sixt 0 siblings, 1 reply; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-26 13:32 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Chris Idema via GitGitGadget > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 13:32 ` GitHub Chris Idema @ 2026-01-26 13:59 ` Johannes Sixt 2026-01-26 14:43 ` GitHub Chris Idema 0 siblings, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-26 13:59 UTC (permalink / raw) To: GitHub Chris Idema; +Cc: git, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 13:59 ` Johannes Sixt @ 2026-01-26 14:43 ` GitHub Chris Idema 2026-01-26 14:52 ` Johannes Sixt 0 siblings, 1 reply; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-26 14:43 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Chris Idema via GitGitGadget > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 14:43 ` GitHub Chris Idema @ 2026-01-26 14:52 ` Johannes Sixt 2026-01-26 15:21 ` GitHub Chris Idema 2026-01-26 15:32 ` GitHub Chris Idema 0 siblings, 2 replies; 30+ messages in thread From: Johannes Sixt @ 2026-01-26 14:52 UTC (permalink / raw) To: GitHub Chris Idema; +Cc: git, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 14:52 ` Johannes Sixt @ 2026-01-26 15:21 ` GitHub Chris Idema 2026-01-26 15:32 ` GitHub Chris Idema 1 sibling, 0 replies; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-26 15:21 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Chris Idema via GitGitGadget > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-26 14:52 ` Johannes Sixt 2026-01-26 15:21 ` GitHub Chris Idema @ 2026-01-26 15:32 ` GitHub Chris Idema 1 sibling, 0 replies; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-26 15:32 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH/RFC v2 0/2] diff.tcl: Fixed alignment of tabs in git-gui diff by using spaces. 2026-01-26 10:45 [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces Chris Idema via GitGitGadget 2026-01-26 12:15 ` Johannes Sixt @ 2026-01-27 20:33 ` Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 1/2] diff.tcl: fixed " Chris Idema via GitGitGadget ` (2 more replies) 1 sibling, 3 replies; 30+ messages in thread From: Chris Idema via GitGitGadget @ 2026-01-27 20:33 UTC (permalink / raw) To: git; +Cc: Chris Idema cc: Johannes Sixt j6t@kdbg.org Chris Idema (2): diff.tcl: fixed alignment of tabs in git-gui diff by using spaces diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. git-gui/lib/diff.tcl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) base-commit: 1faf5b085a171f9ba9a6d7a446e0de16acccb1dc Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2179%2FChrisIdema%2Ffix-gitgui-diff-tab-alignment-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2179/ChrisIdema/fix-gitgui-diff-tab-alignment-v2 Pull-Request: https://github.com/git/git/pull/2179 Range-diff vs v1: 1: f2a09c15eb = 1: f2a09c15eb diff.tcl: fixed alignment of tabs in git-gui diff by using spaces -: ---------- > 2: e11aa6d811 diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. -- gitgitgadget ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH/RFC v2 1/2] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget @ 2026-01-27 20:33 ` Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces Chris Idema via GitGitGadget 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget 2 siblings, 0 replies; 30+ messages in thread From: Chris Idema via GitGitGadget @ 2026-01-27 20:33 UTC (permalink / raw) To: git; +Cc: Chris Idema, Chris Idema From: Chris Idema <github_chris_idema@proton.me> Tabs were not properly rendered in TK regardless of tab width settings. Converting tab alignment to spaces before rendering in TK fixes this. Does not fix alignment issues in gitk. Signed-off-by: Chris Idema <github_chris_idema@proton.me> --- git-gui/lib/diff.tcl | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/git-gui/lib/diff.tcl b/git-gui/lib/diff.tcl index 442737ba4f..2e13f8c776 100644 --- a/git-gui/lib/diff.tcl +++ b/git-gui/lib/diff.tcl @@ -12,6 +12,27 @@ 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 @@ -495,7 +516,9 @@ proc read_diff {fd conflict_size cont_info} { } } set mark [$ui_diff index "end - 1 line linestart"] - $ui_diff insert end $line $tags + set line [expand_tabs $line] + $ui_diff insert end "$line" $tags + if {[string index $line end] eq "\r"} { $ui_diff tag add d_cr {end - 2c} } -- gitgitgadget ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 1/2] diff.tcl: fixed " Chris Idema via GitGitGadget @ 2026-01-27 20:33 ` Chris Idema via GitGitGadget 2026-01-27 22:19 ` Junio C Hamano 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget 2 siblings, 1 reply; 30+ messages in thread From: Chris Idema via GitGitGadget @ 2026-01-27 20:33 UTC (permalink / raw) To: git; +Cc: Chris Idema, Chris Idema 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 - if {[string index $line end] eq "\r"} { $ui_diff tag add d_cr {end - 2c} } -- gitgitgadget ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-27 20:33 ` [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces Chris Idema via GitGitGadget @ 2026-01-27 22:19 ` Junio C Hamano 2026-01-27 23:26 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2026-01-27 22:19 UTC (permalink / raw) To: Chris Idema via GitGitGadget; +Cc: git, Chris Idema "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 ... ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-27 22:19 ` Junio C Hamano @ 2026-01-27 23:26 ` Junio C Hamano 2026-01-28 9:07 ` GitHub Chris Idema 2026-01-28 13:40 ` Johannes Sixt 0 siblings, 2 replies; 30+ messages in thread From: Junio C Hamano @ 2026-01-27 23:26 UTC (permalink / raw) To: Chris Idema via GitGitGadget; +Cc: git, Chris Idema, Michael Lutz, Pat Thoyts 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] ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-27 23:26 ` Junio C Hamano @ 2026-01-28 9:07 ` GitHub Chris Idema 2026-01-28 13:40 ` Johannes Sixt 1 sibling, 0 replies; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-28 9:07 UTC (permalink / raw) To: Junio C Hamano Cc: Chris Idema via GitGitGadget, git, Michael Lutz, Pat Thoyts, Johannes Sixt > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-27 23:26 ` Junio C Hamano 2026-01-28 9:07 ` GitHub Chris Idema @ 2026-01-28 13:40 ` Johannes Sixt 2026-01-28 14:02 ` GitHub Chris Idema 2026-01-29 0:06 ` Junio C Hamano 1 sibling, 2 replies; 30+ messages in thread From: Johannes Sixt @ 2026-01-28 13:40 UTC (permalink / raw) To: Junio C Hamano, Chris Idema Cc: git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-28 13:40 ` Johannes Sixt @ 2026-01-28 14:02 ` GitHub Chris Idema 2026-01-28 15:59 ` Johannes Sixt 2026-01-29 0:06 ` Junio C Hamano 1 sibling, 1 reply; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-28 14:02 UTC (permalink / raw) To: Johannes Sixt Cc: Junio C Hamano, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget >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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-28 14:02 ` GitHub Chris Idema @ 2026-01-28 15:59 ` Johannes Sixt 2026-01-28 23:42 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-28 15:59 UTC (permalink / raw) To: GitHub Chris Idema Cc: Junio C Hamano, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-28 15:59 ` Johannes Sixt @ 2026-01-28 23:42 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2026-01-28 23:42 UTC (permalink / raw) To: Johannes Sixt Cc: GitHub Chris Idema, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-28 13:40 ` Johannes Sixt 2026-01-28 14:02 ` GitHub Chris Idema @ 2026-01-29 0:06 ` Junio C Hamano 2026-01-29 8:31 ` GitHub Chris Idema 1 sibling, 1 reply; 30+ messages in thread From: Junio C Hamano @ 2026-01-29 0:06 UTC (permalink / raw) To: Johannes Sixt Cc: Chris Idema, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-29 0:06 ` Junio C Hamano @ 2026-01-29 8:31 ` GitHub Chris Idema 2026-01-29 10:04 ` Johannes Sixt 0 siblings, 1 reply; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-29 8:31 UTC (permalink / raw) To: Junio C Hamano Cc: Johannes Sixt, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-29 8:31 ` GitHub Chris Idema @ 2026-01-29 10:04 ` Johannes Sixt 2026-01-29 15:17 ` Junio C Hamano 0 siblings, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-29 10:04 UTC (permalink / raw) To: GitHub Chris Idema, Junio C Hamano Cc: git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. 2026-01-29 10:04 ` Johannes Sixt @ 2026-01-29 15:17 ` Junio C Hamano 0 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2026-01-29 15:17 UTC (permalink / raw) To: Johannes Sixt Cc: GitHub Chris Idema, git, Michael Lutz, Pat Thoyts, Chris Idema via GitGitGadget 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk 2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 1/2] diff.tcl: fixed " Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces Chris Idema via GitGitGadget @ 2026-01-28 10:20 ` Chris Idema via GitGitGadget 2026-01-28 17:02 ` Johannes Sixt ` (2 more replies) 2 siblings, 3 replies; 30+ messages in thread From: Chris Idema via GitGitGadget @ 2026-01-28 10:20 UTC (permalink / raw) To: git; +Cc: Chris Idema, Chris Idema 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. 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 Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2179%2FChrisIdema%2Ffix-gitgui-diff-tab-alignment-v3 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2179/ChrisIdema/fix-gitgui-diff-tab-alignment-v3 Pull-Request: https://github.com/git/git/pull/2179 Range-diff vs v2: 1: f2a09c15eb < -: ---------- diff.tcl: fixed alignment of tabs in git-gui diff by using spaces 2: e11aa6d811 ! 1: 18d25b90c4 diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. @@ Metadata Author: Chris Idema <github_chris_idema@proton.me> ## Commit message ## - diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces. + diff.tcl: made alignment of tabs in git-gui diff consistent with gitk + + 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. Signed-off-by: Chris Idema <github_chris_idema@proton.me> ## git-gui/lib/diff.tcl ## -@@ git-gui/lib/diff.tcl: 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 @@ git-gui/lib/diff.tcl: 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 -- + $ui_diff insert end $line $tags if {[string index $line end] eq "\r"} { $ui_diff tag add d_cr {end - 2c} - } 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 -- gitgitgadget ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget @ 2026-01-28 17:02 ` Johannes Sixt 2026-01-28 19:02 ` GitHub Chris Idema 2026-01-29 0:02 ` Junio C Hamano 2026-01-29 11:09 ` [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs Chris Idema via GitGitGadget 2 siblings, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-28 17:02 UTC (permalink / raw) To: Chris Idema; +Cc: Junio C Hamano, Chris Idema via GitGitGadget, git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk 2026-01-28 17:02 ` Johannes Sixt @ 2026-01-28 19:02 ` GitHub Chris Idema 0 siblings, 0 replies; 30+ messages in thread From: GitHub Chris Idema @ 2026-01-28 19:02 UTC (permalink / raw) To: Johannes Sixt; +Cc: Junio C Hamano, Chris Idema via GitGitGadget, git > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget 2026-01-28 17:02 ` Johannes Sixt @ 2026-01-29 0:02 ` Junio C Hamano 2026-01-29 11:09 ` [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs Chris Idema via GitGitGadget 2 siblings, 0 replies; 30+ messages in thread From: Junio C Hamano @ 2026-01-29 0:02 UTC (permalink / raw) To: Chris Idema via GitGitGadget; +Cc: git, Chris Idema "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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget 2026-01-28 17:02 ` Johannes Sixt 2026-01-29 0:02 ` Junio C Hamano @ 2026-01-29 11:09 ` Chris Idema via GitGitGadget 2026-01-29 21:36 ` Johannes Sixt 2 siblings, 1 reply; 30+ messages in thread From: Chris Idema via GitGitGadget @ 2026-01-29 11:09 UTC (permalink / raw) To: git; +Cc: Chris Idema, Chris Idema 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: shift tabstops to account for the first column of context diffs cc: Johannes Sixt j6t@kdbg.org Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2179%2FChrisIdema%2Ffix-gitgui-diff-tab-alignment-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2179/ChrisIdema/fix-gitgui-diff-tab-alignment-v4 Pull-Request: https://github.com/git/git/pull/2179 Range-diff vs v3: 1: 18d25b90c4 ! 1: 1c91363568 diff.tcl: made alignment of tabs in git-gui diff consistent with gitk @@ Metadata Author: Chris Idema <github_chris_idema@proton.me> ## Commit message ## - diff.tcl: made alignment of tabs in git-gui diff consistent with gitk + git-gui: shift tabstops to account for the first column of context diffs - 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. + 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 ## @@ git-gui/lib/diff.tcl: 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 } - 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} + 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 } base-commit: 1faf5b085a171f9ba9a6d7a446e0de16acccb1dc -- gitgitgadget ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs 2026-01-29 11:09 ` [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs Chris Idema via GitGitGadget @ 2026-01-29 21:36 ` Johannes Sixt 2026-03-04 13:32 ` GitHub Chris Idema 0 siblings, 1 reply; 30+ messages in thread From: Johannes Sixt @ 2026-01-29 21:36 UTC (permalink / raw) To: Chris Idema; +Cc: Chris Idema via GitGitGadget, git 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs 2026-01-29 21:36 ` Johannes Sixt @ 2026-03-04 13:32 ` GitHub Chris Idema 2026-03-04 19:22 ` Johannes Sixt 0 siblings, 1 reply; 30+ messages in thread From: GitHub Chris Idema @ 2026-03-04 13:32 UTC (permalink / raw) To: Johannes Sixt; +Cc: Chris Idema via GitGitGadget, git Any updates? I see it hasn't been merged yet. Chris Idema -------- Original Message -------- On Thursday, 01/29/26 at 22:36 Johannes Sixt <j6t@kdbg.org> wrote: 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs 2026-03-04 13:32 ` GitHub Chris Idema @ 2026-03-04 19:22 ` Johannes Sixt 0 siblings, 0 replies; 30+ messages in thread From: Johannes Sixt @ 2026-03-04 19:22 UTC (permalink / raw) To: GitHub Chris Idema; +Cc: Chris Idema via GitGitGadget, git Am 04.03.26 um 14:32 schrieb GitHub Chris Idema: > Any updates? I see it hasn't been merged yet. See https://github.com/j6t/git-gui/commits/master/. I'll send the patch(es) upstream in due time. -- Hannes ^ permalink raw reply [flat|nested] 30+ messages in thread
end of thread, other threads:[~2026-03-04 19:22 UTC | newest] Thread overview: 30+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-01-26 10:45 [PATCH] diff.tcl: fixed alignment of tabs in git-gui diff by using spaces Chris Idema via GitGitGadget 2026-01-26 12:15 ` Johannes Sixt 2026-01-26 13:32 ` GitHub Chris Idema 2026-01-26 13:59 ` Johannes Sixt 2026-01-26 14:43 ` GitHub Chris Idema 2026-01-26 14:52 ` Johannes Sixt 2026-01-26 15:21 ` GitHub Chris Idema 2026-01-26 15:32 ` GitHub Chris Idema 2026-01-27 20:33 ` [PATCH/RFC v2 0/2] diff.tcl: Fixed " Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 1/2] diff.tcl: fixed " Chris Idema via GitGitGadget 2026-01-27 20:33 ` [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces Chris Idema via GitGitGadget 2026-01-27 22:19 ` Junio C Hamano 2026-01-27 23:26 ` Junio C Hamano 2026-01-28 9:07 ` GitHub Chris Idema 2026-01-28 13:40 ` Johannes Sixt 2026-01-28 14:02 ` GitHub Chris Idema 2026-01-28 15:59 ` Johannes Sixt 2026-01-28 23:42 ` Junio C Hamano 2026-01-29 0:06 ` Junio C Hamano 2026-01-29 8:31 ` GitHub Chris Idema 2026-01-29 10:04 ` Johannes Sixt 2026-01-29 15:17 ` Junio C Hamano 2026-01-28 10:20 ` [PATCH/RFC v3] diff.tcl: made alignment of tabs in git-gui diff consistent with gitk Chris Idema via GitGitGadget 2026-01-28 17:02 ` Johannes Sixt 2026-01-28 19:02 ` GitHub Chris Idema 2026-01-29 0:02 ` Junio C Hamano 2026-01-29 11:09 ` [PATCH v4] git-gui: shift tabstops to account for the first column of context diffs Chris Idema via GitGitGadget 2026-01-29 21:36 ` Johannes Sixt 2026-03-04 13:32 ` GitHub Chris Idema 2026-03-04 19:22 ` Johannes Sixt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox