Re: [PATCH/RFC v2 2/2] diff.tcl: call "apply_tab_size 1" to fix alignment instead of spaces.
Junio C Hamano <gitster@pobox.com> writes:
Show 27 quoted lines
> 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>Show changes to 3 files +21 −1
git-gui.sh, lib/diff.tcl, lib/option.tcl
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]
