threads / patch / 9238

patchgitk: let you easily specify lines of context in diff view

Subject: [PATCH] gitk: let you easily specify lines of context in diff view

## tl;dr

10 messages between Jul 26, 2007 and Aug 12, 2007. Diffs are folded; open one to read it.

replies: 9people: 2as markdown or json

Steffen Prohaska· Jul 26, 2007, 07:59 UTC · lore

More lines of context sometimes help to better understand a diff. This patch introduces a text field above the box displaying the blobdiffs. You can type in the number of lines of context that you wish to view.

Minor improvements are needed:
   * The value you type in is only used on the next update.
   * lines of context is initially always 3.

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 gitk |   25 +++++++++++++++++++++++--
 1 files changed, 23 insertions(+), 2 deletions(-)

The patch may need some polishing. I know some tcl but this is the 
first time I tried tk:
   * I initialized diffcontext globally, ok?
   * I don't know how to update the view after entering a new value. 
  
    Steffen

Show changes to gitk +23 −2

diff --git a/gitk b/gitk
index 39e452a..4e9acff 100755
--- a/gitk
+++ b/gitk
@@ -500,6 +500,7 @@ proc makewindow {} {
     global textfont mainfont uifont tabstop
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
+    global diffcontextstring
     global maincursor textcursor curtextcursor
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
@@ -714,7 +715,12 @@ proc makewindow {} {
 	-command changediffdisp -variable diffelide -value {0 1}
     radiobutton .bleft.mid.new -text "New version" \
 	-command changediffdisp -variable diffelide -value {1 0}
-    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
+    -font $uifont
+    entry .bleft.mid.diffcontext -width 5 -font $textfont -textvariable diffcontextstring
+    trace add variable diffcontextstring write diffcontextchange
+    lappend entries .bleft.mid.diffcontext
+    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-tabs "[expr {$tabstop * $charspc}]" \
@@ -4872,12 +4878,25 @@ proc gettreediffline {gdtf ids} {
     return 0
 }
 
+proc diffcontextchange {n1 n2 op} {
+    global diffcontextstring diffcontext
+
+    if {[string is integer $diffcontextstring]} {
+        if {$diffcontextstring > 0} {
+            set diffcontext $diffcontextstring
+# TODO: need to trigger update of diff display
+# tried dodiffindex but that corrupted the history view
+        }
+    }
+}
+
 proc getblobdiffs {ids} {
     global diffopts blobdifffd diffids env
     global diffinhdr treediffs
+    global diffcontext
 
     set env(GIT_DIFF_OPTS) $diffopts
-    if {[catch {set bdf [open [diffcmd $ids {-p -C}] r]} err]} {
+    if {[catch {set bdf [open [diffcmd $ids "-p -C -U$diffcontext"] r]} err]} {
 	puts "error getting diffs: $err"
 	return
     }
@@ -7537,6 +7556,8 @@ set markingmatches 0
 
 set optim_delay 16
 
+set diffcontext 3 
+
 set nextviewnum 1
 set curview 0
 set selectedview 0

-- 
1.5.3.rc3.20.g06b4

Paul Mackerras· Jul 26, 2007, 12:34 UTC · re: Steffen Prohaska · lore

Re: [PATCH] gitk: let you easily specify lines of context in diff view

Steffen Prohaska writes:

> More lines of context sometimes help to better understand a diff.
> This patch introduces a text field above the box displaying the
> blobdiffs. You can type in the number of lines of context that
> you wish to view.

Nice idea! I suggest you use a spinbox instead of an entry though, since that has up and down buttons, and allows you to restrict the value to an integer.

>    * I don't know how to update the view after entering a new value.

You can put a trace on the associated variable and specify a function to be called when the variable's value changes. Grep for "trace add variable" in gitk to see how it's done.

Paul.

Steffen Prohaska· Jul 27, 2007, 05:52 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: let you easily specify lines of context in diff view

On Jul 26, 2007, at 2:34 PM, Paul Mackerras wrote:

Show 10 quoted lines

> Steffen Prohaska writes:
>
>> More lines of context sometimes help to better understand a diff.
>> This patch introduces a text field above the box displaying the
>> blobdiffs. You can type in the number of lines of context that
>> you wish to view.
>
> Nice idea!  I suggest you use a spinbox instead of an entry though,
> since that has up and down buttons, and allows you to restrict the
> value to an integer.

Thanks. Will try that.

Show 5 quoted lines

>>    * I don't know how to update the view after entering a new value.
>
> You can put a trace on the associated variable and specify a function
> to be called when the variable's value changes.  Grep for "trace add
> variable" in gitk to see how it's done.

This is already included in the patch but I don't know which of gtk's procs to call to update the view. gitk would need to execute git diff and update the text box in the bottom left. I tried 'dodiffindex' but this causes corruptions of the history view.

	Steffen

Paul Mackerras· Jul 27, 2007, 10:31 UTC · re: Steffen Prohaska · lore

Re: [PATCH] gitk: let you easily specify lines of context in diff view

Steffen Prohaska writes:

> This is already included in the patch but I don't know which of gtk's
> procs to call to update the view. gitk would need to execute git diff
> and update the text box in the bottom left. I tried 'dodiffindex'
> but this causes corruptions of the history view.

reselectline should do what you want.

Paul.

Steffen Prohaska· Jul 27, 2007, 11:44 UTC · re: Paul Mackerras · lore

[PATCH v2] gitk: let you easily specify lines of context in diff view

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 gitk |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)

I included the changes proposed by Paul. From my side, the patch is ready to be applied.

    Steffen

Show changes to gitk +39 −2

diff --git a/gitk b/gitk
index 39e452a..9de9574 100755
--- a/gitk
+++ b/gitk
@@ -500,6 +500,7 @@ proc makewindow {} {
     global textfont mainfont uifont tabstop
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
+    global diffcontextstring diffcontext
     global maincursor textcursor curtextcursor
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
@@ -714,7 +715,16 @@ proc makewindow {} {
 	-command changediffdisp -variable diffelide -value {0 1}
     radiobutton .bleft.mid.new -text "New version" \
 	-command changediffdisp -variable diffelide -value {1 0}
-    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
+    -font $uifont
+    spinbox .bleft.mid.diffcontext -width 5 -font $textfont \
+    -from 1 -increment 1 -to 10000000 \
+    -validate all -validatecommand "diffcontextvalidate %P" \
+    -textvariable diffcontextstring
+    .bleft.mid.diffcontext set $diffcontext
+    trace add variable diffcontextstring write diffcontextchange
+    lappend entries .bleft.mid.diffcontext
+    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-tabs "[expr {$tabstop * $charspc}]" \
@@ -4872,12 +4882,37 @@ proc gettreediffline {gdtf ids} {
     return 0
 }
 
+# empty strings or integers accepted
+proc diffcontextvalidate {v} {
+    if {[string length $v] == 0} {
+	return 1
+    }
+    if {[string is integer $v]} {
+	if {$v > 0} {
+	    return 1
+	}
+    }
+    return 0
+}
+
+proc diffcontextchange {n1 n2 op} {
+    global diffcontextstring diffcontext
+
+    if {[string is integer $diffcontextstring]} {
+        if {$diffcontextstring > 0} {
+            set diffcontext $diffcontextstring
+		    reselectline
+        }
+    }
+}
+
 proc getblobdiffs {ids} {
     global diffopts blobdifffd diffids env
     global diffinhdr treediffs
+    global diffcontext
 
     set env(GIT_DIFF_OPTS) $diffopts
-    if {[catch {set bdf [open [diffcmd $ids {-p -C}] r]} err]} {
+    if {[catch {set bdf [open [diffcmd $ids "-p -C -U$diffcontext"] r]} err]} {
 	puts "error getting diffs: $err"
 	return
     }
@@ -7537,6 +7572,8 @@ set markingmatches 0
 
 set optim_delay 16
 
+set diffcontext 3 
+
 set nextviewnum 1
 set curview 0
 set selectedview 0

-- 
1.5.3.rc3.21.g45610

Steffen Prohaska· Jul 28, 2007, 19:18 UTC · re: Steffen Prohaska · lore

[PATCH v3] gitk: let you easily specify lines of context in diff view

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 gitk |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)

rebased to the current master.

    Steffen

Show changes to gitk +39 −2

diff --git a/gitk b/gitk
index f74ce51..670a162 100755
--- a/gitk
+++ b/gitk
@@ -517,6 +517,7 @@ proc makewindow {} {
     global textfont mainfont uifont tabstop
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
+    global diffcontextstring diffcontext
     global maincursor textcursor curtextcursor
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
@@ -731,7 +732,16 @@ proc makewindow {} {
 	-command changediffdisp -variable diffelide -value {0 1}
     radiobutton .bleft.mid.new -text "New version" \
 	-command changediffdisp -variable diffelide -value {1 0}
-    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
+    -font $uifont
+    spinbox .bleft.mid.diffcontext -width 5 -font $textfont \
+    -from 1 -increment 1 -to 10000000 \
+    -validate all -validatecommand "diffcontextvalidate %P" \
+    -textvariable diffcontextstring
+    .bleft.mid.diffcontext set $diffcontext
+    trace add variable diffcontextstring write diffcontextchange
+    lappend entries .bleft.mid.diffcontext
+    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-tabs "[expr {$tabstop * $charspc}]" \
@@ -4985,12 +4995,37 @@ proc gettreediffline {gdtf ids} {
     return 0
 }
 
+# empty strings or integers accepted
+proc diffcontextvalidate {v} {
+    if {[string length $v] == 0} {
+	return 1
+    }
+    if {[string is integer $v]} {
+	if {$v > 0} {
+	    return 1
+	}
+    }
+    return 0
+}
+
+proc diffcontextchange {n1 n2 op} {
+    global diffcontextstring diffcontext
+
+    if {[string is integer $diffcontextstring]} {
+        if {$diffcontextstring > 0} {
+            set diffcontext $diffcontextstring
+		    reselectline
+        }
+    }
+}
+
 proc getblobdiffs {ids} {
     global diffopts blobdifffd diffids env
     global diffinhdr treediffs
+    global diffcontext
 
     set env(GIT_DIFF_OPTS) $diffopts
-    if {[catch {set bdf [open [diffcmd $ids {-p -C --no-commit-id}] r]} err]} {
+    if {[catch {set bdf [open [diffcmd $ids "-p -C --no-commit-id -U$diffcontext"] r]} err]} {
 	puts "error getting diffs: $err"
 	return
     }
@@ -7646,6 +7681,8 @@ set markingmatches 0
 
 set optim_delay 16
 
+set diffcontext 3 
+
 set nextviewnum 1
 set curview 0
 set selectedview 0

-- 
1.5.3.rc3.45.g4c741

Steffen Prohaska· Aug 11, 2007, 15:30 UTC · re: Steffen Prohaska · lore

[PATCH v4, ping] gitk: let you easily specify lines of context in diff view

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 gitk |   41 +++++++++++++++++++++++++++++++++++++++--
 1 files changed, 39 insertions(+), 2 deletions(-)

rebased to the current master, fixed one trailing whitespace.

Any chance to get this patch applied? It works for me.

    Steffen

Show changes to gitk +39 −2

diff --git a/gitk b/gitk
index f74ce51..2f86c7c 100755
--- a/gitk
+++ b/gitk
@@ -517,6 +517,7 @@ proc makewindow {} {
     global textfont mainfont uifont tabstop
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
+    global diffcontextstring diffcontext
     global maincursor textcursor curtextcursor
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
@@ -731,7 +732,16 @@ proc makewindow {} {
 	-command changediffdisp -variable diffelide -value {0 1}
     radiobutton .bleft.mid.new -text "New version" \
 	-command changediffdisp -variable diffelide -value {1 0}
-    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
+    -font $uifont
+    spinbox .bleft.mid.diffcontext -width 5 -font $textfont \
+    -from 1 -increment 1 -to 10000000 \
+    -validate all -validatecommand "diffcontextvalidate %P" \
+    -textvariable diffcontextstring
+    .bleft.mid.diffcontext set $diffcontext
+    trace add variable diffcontextstring write diffcontextchange
+    lappend entries .bleft.mid.diffcontext
+    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-tabs "[expr {$tabstop * $charspc}]" \
@@ -4985,12 +4995,37 @@ proc gettreediffline {gdtf ids} {
     return 0
 }
 
+# empty strings or integers accepted
+proc diffcontextvalidate {v} {
+    if {[string length $v] == 0} {
+	return 1
+    }
+    if {[string is integer $v]} {
+	if {$v > 0} {
+	    return 1
+	}
+    }
+    return 0
+}
+
+proc diffcontextchange {n1 n2 op} {
+    global diffcontextstring diffcontext
+
+    if {[string is integer $diffcontextstring]} {
+        if {$diffcontextstring > 0} {
+            set diffcontext $diffcontextstring
+		    reselectline
+        }
+    }
+}
+
 proc getblobdiffs {ids} {
     global diffopts blobdifffd diffids env
     global diffinhdr treediffs
+    global diffcontext
 
     set env(GIT_DIFF_OPTS) $diffopts
-    if {[catch {set bdf [open [diffcmd $ids {-p -C --no-commit-id}] r]} err]} {
+    if {[catch {set bdf [open [diffcmd $ids "-p -C --no-commit-id -U$diffcontext"] r]} err]} {
 	puts "error getting diffs: $err"
 	return
     }
@@ -7646,6 +7681,8 @@ set markingmatches 0
 
 set optim_delay 16
 
+set diffcontext 3
+
 set nextviewnum 1
 set curview 0
 set selectedview 0

-- 
1.5.3.rc4.31.g6adda

Paul Mackerras· Aug 12, 2007, 03:17 UTC · re: Steffen Prohaska · lore

Re: [PATCH v4, ping] gitk: let you easily specify lines of context in diff view

Steffen Prohaska writes:

> Any chance to get this patch applied? It works for me.

Some comments:

Show 5 quoted lines

> @@ -731,7 +732,16 @@ proc makewindow {} {
>  	-command changediffdisp -variable diffelide -value {0 1}
>      radiobutton .bleft.mid.new -text "New version" \
>  	-command changediffdisp -variable diffelide -value {1 0}
> -    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left

Just add another pack command rather than extending this one.

> +    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
> +    -font $uifont

This is hard to read because the continuation line isn't indented further that the first line. Please indent continuation lines by an extra 4 spaces.

Show 12 quoted lines

> +# empty strings or integers accepted
> +proc diffcontextvalidate {v} {
> +    if {[string length $v] == 0} {
> +	return 1
> +    }
> +    if {[string is integer $v]} {
> +	if {$v > 0} {
> +	    return 1
> +	}
> +    }
> +    return 0
> +}

"string is integer" will already accept the null string and return 1.

Show 5 quoted lines

> +proc diffcontextchange {n1 n2 op} {
> +    global diffcontextstring diffcontext
> +
> +    if {[string is integer $diffcontextstring]} {
> +        if {$diffcontextstring > 0} {

Once again, "string is integer" returning 1 doesn't guarantee the string is non-empty. Use "string is integer -strict" if you want that.

> +            set diffcontext $diffcontextstring
> +		    reselectline

Inconsistent indentation.

> +set diffcontext 3
> +

It would be nice to save diffcontext in the ~/.gitkrc.

Paul.

Steffen Prohaska· Aug 12, 2007, 10:02 UTC · re: Paul Mackerras · lore

Re: [PATCH v4, ping] gitk: let you easily specify lines of context in diff view

On Aug 12, 2007, at 5:17 AM, Paul Mackerras wrote:

Show 13 quoted lines

> Steffen Prohaska writes:
>
>> Any chance to get this patch applied? It works for me.
>
> Some comments:
>
>> @@ -731,7 +732,16 @@ proc makewindow {} {
>>  	-command changediffdisp -variable diffelide -value {0 1}
>>      radiobutton .bleft.mid.new -text "New version" \
>>  	-command changediffdisp -variable diffelide -value {1 0}
>> -    pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
>
> Just add another pack command rather than extending this one.

Thanks. Didn't understand what pack does and thought it might be needed to pack all at once into a 'single' pack.

Show 7 quoted lines

>> +    label .bleft.mid.labeldiffcontext -text "      Lines of  
>> context: " \
>> +    -font $uifont
>
> This is hard to read because the continuation line isn't indented
> further that the first line.  Please indent continuation lines by an
> extra 4 spaces.

I fixed more of them. I typically work with a no-tab rule. Hope I spotted all other places with wrong indent, too.

Show 14 quoted lines

>> +# empty strings or integers accepted
>> +proc diffcontextvalidate {v} {
>> +    if {[string length $v] == 0} {
>> +	return 1
>> +    }
>> +    if {[string is integer $v]} {
>> +	if {$v > 0} {
>> +	    return 1
>> +	}
>> +    }
>> +    return 0
>> +}
>
> "string is integer" will already accept the null string and return 1.

replaced this by a regexp.

Show 9 quoted lines

>> +proc diffcontextchange {n1 n2 op} {
>> +    global diffcontextstring diffcontext
>> +
>> +    if {[string is integer $diffcontextstring]} {
>> +        if {$diffcontextstring > 0} {
>
> Once again, "string is integer" returning 1 doesn't guarantee the
> string is non-empty.  Use "string is integer -strict" if you want
> that.

Ah, thanks, didn't know that.

>> +            set diffcontext $diffcontextstring
>> +		    reselectline
>
> Inconsistent indentation.

ok.

>> +set diffcontext 3
>> +
>
> It would be nice to save diffcontext in the ~/.gitkrc.

done.

Will send a patch in a minute.

	Steffen

Steffen Prohaska· Aug 12, 2007, 10:05 UTC · re: Steffen Prohaska · lore

[PATCH v5] gitk: let you easily specify lines of context in diff view

Signed-off-by: Steffen Prohaska <prohaska@zib.de>
---
 gitk |   34 ++++++++++++++++++++++++++++++++--
 1 files changed, 32 insertions(+), 2 deletions(-)

Show changes to gitk +32 −2

diff --git a/gitk b/gitk
index 1103baf..4366c7f 100755
--- a/gitk
+++ b/gitk
@@ -517,6 +517,7 @@ proc makewindow {} {
     global textfont mainfont uifont tabstop
     global findtype findtypemenu findloc findstring fstring geometry
     global entries sha1entry sha1string sha1but
+    global diffcontextstring diffcontext
     global maincursor textcursor curtextcursor
     global rowctxmenu fakerowmenu mergemax wrapcomment
     global highlight_files gdttype
@@ -731,7 +732,17 @@ proc makewindow {} {
 	-command changediffdisp -variable diffelide -value {0 1}
     radiobutton .bleft.mid.new -text "New version" \
 	-command changediffdisp -variable diffelide -value {1 0}
+    label .bleft.mid.labeldiffcontext -text "      Lines of context: " \
+	-font $uifont
     pack .bleft.mid.diff .bleft.mid.old .bleft.mid.new -side left
+    spinbox .bleft.mid.diffcontext -width 5 -font $textfont \
+	-from 1 -increment 1 -to 10000000 \
+	-validate all -validatecommand "diffcontextvalidate %P" \
+	-textvariable diffcontextstring
+    .bleft.mid.diffcontext set $diffcontext
+    trace add variable diffcontextstring write diffcontextchange
+    lappend entries .bleft.mid.diffcontext
+    pack .bleft.mid.labeldiffcontext .bleft.mid.diffcontext -side left
     set ctext .bleft.ctext
     text $ctext -background $bgcolor -foreground $fgcolor \
 	-tabs "[expr {$tabstop * $charspc}]" \
@@ -968,7 +979,7 @@ proc savestuff {w} {
     global maxwidth showneartags showlocalchanges
     global viewname viewfiles viewargs viewperm nextviewnum
     global cmitmode wrapcomment
-    global colors bgcolor fgcolor diffcolors selectbgcolor
+    global colors bgcolor fgcolor diffcolors diffcontext selectbgcolor
 
     if {$stuffsaved} return
     if {![winfo viewable .]} return
@@ -989,6 +1000,7 @@ proc savestuff {w} {
 	puts $f [list set fgcolor $fgcolor]
 	puts $f [list set colors $colors]
 	puts $f [list set diffcolors $diffcolors]
+	puts $f [list set diffcontext $diffcontext]
 	puts $f [list set selectbgcolor $selectbgcolor]
 
 	puts $f "set geometry(main) [wm geometry .]"
@@ -4986,12 +4998,29 @@ proc gettreediffline {gdtf ids} {
     return 0
 }
 
+# empty string or positive integer
+proc diffcontextvalidate {v} {
+    return [regexp {^(|[1-9][0-9]*)$} $v]
+}
+
+proc diffcontextchange {n1 n2 op} {
+    global diffcontextstring diffcontext
+
+    if {[string is integer -strict $diffcontextstring]} {
+	if {$diffcontextstring > 0} {
+	    set diffcontext $diffcontextstring
+	    reselectline
+	}
+    }
+}
+
 proc getblobdiffs {ids} {
     global diffopts blobdifffd diffids env
     global diffinhdr treediffs
+    global diffcontext
 
     set env(GIT_DIFF_OPTS) $diffopts
-    if {[catch {set bdf [open [diffcmd $ids {-p -C --no-commit-id}] r]} err]} {
+    if {[catch {set bdf [open [diffcmd $ids "-p -C --no-commit-id -U$diffcontext"] r]} err]} {
 	puts "error getting diffs: $err"
 	return
     }
@@ -7569,6 +7598,7 @@ set colors {green red blue magenta darkgrey brown orange}
 set bgcolor white
 set fgcolor black
 set diffcolors {red "#00a000" blue}
+set diffcontext 3
 set selectbgcolor gray85
 
 catch {source ~/.gitk}

-- 
1.5.3.rc4.762.g17d7