[PATCH 0/2] gitk: fix --full-diff handling

[PATCH 0/2] gitk: fix --full-diff handling

[Felipe Contreras]

Hi,

I find usel to do 'git log --full-duff -- file' to find out all the commits
that touched the file, and show the full diff (not just the one of the file).

Unfortunately gitk doesn't honour this option; the diff is limited in the UI.

The following patches fix that.

Felipe Contreras (2):
  gitk: simplify file filtering
  gitk: handle --full-diff correctly

 gitk-git/gitk | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

-- 
1.8.0.rc2.7.g0961fdf.dirty

[PATCH 1/2] gitk: simplify file filtering

[Felipe Contreras]

git diff is perfectly able to do this with '-- files', no need for
manual filtering.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 gitk-git/gitk | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..b79dfdf 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -7519,9 +7519,13 @@ proc diffcmd {ids flags} {
 }
 
 proc gettreediffs {ids} {
-    global treediff treepending
+    global treediff treepending limitdiffs vfilelimit curview
 
-    if {[catch {set gdtf [open [diffcmd $ids {--no-commit-id}] r]}]} return
+    set cmd [diffcmd $ids {--no-commit-id}]
+    if {$limitdiffs && $vfilelimit($curview) ne {}} {
+	    set cmd [concat $cmd -- $vfilelimit($curview)]
+    }
+    if {[catch {set gdtf [open $cmd r]}]} return
 
     set treepending $ids
     set treediff {}
@@ -7565,17 +7569,7 @@ proc gettreediffline {gdtf ids} {
 	return [expr {$nr >= $max? 2: 1}]
     }
     close $gdtf
-    if {$limitdiffs && $vfilelimit($curview) ne {}} {
-	set flist {}
-	foreach f $treediff {
-	    if {[path_filter $vfilelimit($curview) $f]} {
-		lappend flist $f
-	    }
-	}
-	set treediffs($ids) $flist
-    } else {
-	set treediffs($ids) $treediff
-    }
+    set treediffs($ids) $treediff
     unset treepending
     if {$cmitmode eq "tree" && [llength $diffids] == 1} {
 	gettree $diffids
-- 
1.8.0.rc2.7.g0961fdf.dirty

[PATCH 2/2] gitk: handle --full-diff correctly

[Felipe Contreras]

Otherwise the files are missing from the diff, and the list of files.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 gitk-git/gitk | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index b79dfdf..8109eed 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -155,7 +155,7 @@ proc unmerged_files {files} {
 }
 
 proc parseviewargs {n arglist} {
-    global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
+    global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs vfileargs env
     global worddiff git_version
 
     set vdatemode($n) 0
@@ -165,6 +165,7 @@ proc parseviewargs {n arglist} {
     set nextisval 0
     set revargs {}
     set origargs $arglist
+    set fileargs {}
     set allknown 1
     set filtered 0
     set i -1
@@ -187,7 +188,7 @@ proc parseviewargs {n arglist} {
 	    "--no-renames" - "--full-index" - "--binary" - "--abbrev=*" -
 	    "--find-copies-harder" - "-l*" - "--ext-diff" - "--no-ext-diff" -
 	    "--src-prefix=*" - "--dst-prefix=*" - "--no-prefix" -
-	    "-O*" - "--text" - "--full-diff" - "--ignore-space-at-eol" -
+	    "-O*" - "--text" - "--ignore-space-at-eol" -
 	    "--ignore-space-change" - "-U*" - "--unified=*" {
 		# These request or affect diff output, which we don't want.
 		# Some could be used to set our defaults for diff display.
@@ -233,6 +234,9 @@ proc parseviewargs {n arglist} {
 		set filtered 1
 		lappend glflags $arg
 	    }
+	    "--full-diff" {
+		lappend fileargs $arg
+	    }
 	    "-n" {
 		# This appears to be the only one that has a value as a
 		# separate word following it
@@ -276,6 +280,7 @@ proc parseviewargs {n arglist} {
     set vrevs($n) $revargs
     set vfiltered($n) $filtered
     set vorigargs($n) $origargs
+    set vfileargs($n) $fileargs
     return $allknown
 }
 
@@ -7519,10 +7524,11 @@ proc diffcmd {ids flags} {
 }
 
 proc gettreediffs {ids} {
-    global treediff treepending limitdiffs vfilelimit curview
+    global treediff treepending limitdiffs vfilelimit vfileargs curview
 
     set cmd [diffcmd $ids {--no-commit-id}]
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
+	    set cmd [concat $cmd $vfileargs($curview)]
 	    set cmd [concat $cmd -- $vfilelimit($curview)]
     }
     if {[catch {set gdtf [open $cmd r]}]} return
@@ -7613,7 +7619,7 @@ proc getblobdiffs {ids} {
     global diffcontext
     global ignorespace
     global worddiff
-    global limitdiffs vfilelimit curview
+    global limitdiffs vfilelimit vfileargs curview
     global diffencoding targetline diffnparents
     global git_version currdiffsubmod
 
@@ -7633,6 +7639,7 @@ proc getblobdiffs {ids} {
 	append cmd " --word-diff=porcelain"
     }
     if {$limitdiffs && $vfilelimit($curview) ne {}} {
+	set cmd [concat $cmd $vfileargs($curview)]
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
     if {[catch {set bdf [open $cmd r]} err]} {
-- 
1.8.0.rc2.7.g0961fdf.dirty

Re: [PATCH 0/2] gitk: fix --full-diff handling

[Johannes Sixt]

Am 10/19/2012 12:56, schrieb Felipe Contreras:
> I find usel to do 'git log --full-duff -- file' to find out all the commits
> that touched the file, and show the full diff (not just the one of the file).
> 
> Unfortunately gitk doesn't honour this option; the diff is limited in the UI.

There is Edit->Preferences->General->Limit diff to listed paths. Doesn't
it do what you want if you switch it off?

-- Hannes

Re: [PATCH 0/2] gitk: fix --full-diff handling

[Felipe Contreras]

On Fri, Oct 19, 2012 at 3:12 PM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 10/19/2012 12:56, schrieb Felipe Contreras:
>> I find usel to do 'git log --full-duff -- file' to find out all the commits
>> that touched the file, and show the full diff (not just the one of the file).
>>
>> Unfortunately gitk doesn't honour this option; the diff is limited in the UI.
>
> There is Edit->Preferences->General->Limit diff to listed paths. Doesn't
> it do what you want if you switch it off?

Hmm, I guess so, but it's not triggered from the command line. Maybe
the --full-diff option should enable that flag.

-- 
Felipe Contreras

[PATCH 0/2] gitk: handle --full-diff corectly

[Felipe Contreras]

Basically $SUBJECT. See details in the 2nd patch.

Felipe Contreras (2):
  gitk: simplify file filtering
  gitk: handle --full-diff correctly

 gitk-git/gitk | 36 +++++++++++++++++-------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

-- 
1.8.0

[PATCH 2/2] gitk: handle --full-diff correctly

[Felipe Contreras]

Otherwise the files are missing from the diff, and the list of files.

We do this by creating a limitdiffs variable specific for the view which
can be turned of by using --full-diff.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 gitk-git/gitk | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index b79dfdf..4ffabdf 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -155,11 +155,12 @@ proc unmerged_files {files} {
 }
 
 proc parseviewargs {n arglist} {
-    global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env
-    global worddiff git_version
+    global vdatemode vmergeonly vlimitdiffs vflags vdflags vrevs vfiltered vorigargs env
+    global limitdiffs worddiff git_version
 
     set vdatemode($n) 0
     set vmergeonly($n) 0
+    set vlimitdiffs($n) $limitdiffs
     set glflags {}
     set diffargs {}
     set nextisval 0
@@ -187,7 +188,7 @@ proc parseviewargs {n arglist} {
 	    "--no-renames" - "--full-index" - "--binary" - "--abbrev=*" -
 	    "--find-copies-harder" - "-l*" - "--ext-diff" - "--no-ext-diff" -
 	    "--src-prefix=*" - "--dst-prefix=*" - "--no-prefix" -
-	    "-O*" - "--text" - "--full-diff" - "--ignore-space-at-eol" -
+	    "-O*" - "--text" - "--ignore-space-at-eol" -
 	    "--ignore-space-change" - "-U*" - "--unified=*" {
 		# These request or affect diff output, which we don't want.
 		# Some could be used to set our defaults for diff display.
@@ -251,6 +252,9 @@ proc parseviewargs {n arglist} {
 	    "--no-replace-objects" {
 		set env(GIT_NO_REPLACE_OBJECTS) "1"
 	    }
+	    "--full-diff" {
+		set vlimitdiffs($n) 0
+	    }
 	    "-*" {
 		# Other flag arguments including -<n>
 		if {[string is digit -strict [string range $arg 1 end]]} {
@@ -7519,10 +7523,10 @@ proc diffcmd {ids flags} {
 }
 
 proc gettreediffs {ids} {
-    global treediff treepending limitdiffs vfilelimit curview
+    global treediff treepending vlimitdiffs vfilelimit curview
 
     set cmd [diffcmd $ids {--no-commit-id}]
-    if {$limitdiffs && $vfilelimit($curview) ne {}} {
+    if {$vlimitdiffs($curview) && $vfilelimit($curview) ne {}} {
 	    set cmd [concat $cmd -- $vfilelimit($curview)]
     }
     if {[catch {set gdtf [open $cmd r]}]} return
@@ -7535,7 +7539,7 @@ proc gettreediffs {ids} {
 
 proc gettreediffline {gdtf ids} {
     global treediff treediffs treepending diffids diffmergeid
-    global cmitmode vfilelimit curview limitdiffs perfile_attrs
+    global cmitmode vfilelimit curview perfile_attrs
 
     set nr 0
     set sublist {}
@@ -7613,7 +7617,7 @@ proc getblobdiffs {ids} {
     global diffcontext
     global ignorespace
     global worddiff
-    global limitdiffs vfilelimit curview
+    global vlimitdiffs vfilelimit curview
     global diffencoding targetline diffnparents
     global git_version currdiffsubmod
 
@@ -7632,7 +7636,7 @@ proc getblobdiffs {ids} {
     if {$worddiff ne [mc "Line diff"]} {
 	append cmd " --word-diff=porcelain"
     }
-    if {$limitdiffs && $vfilelimit($curview) ne {}} {
+    if {$vlimitdiffs($curview) && $vfilelimit($curview) ne {}} {
 	set cmd [concat $cmd -- $vfilelimit($curview)]
     }
     if {[catch {set bdf [open $cmd r]} err]} {
-- 
1.8.0

Re: [PATCH 2/2] gitk: handle --full-diff correctly

[Johannes Sixt]

Am 10/25/2012 19:23, schrieb Felipe Contreras:
> Otherwise the files are missing from the diff, and the list of files.
> 
> We do this by creating a limitdiffs variable specific for the view which
> can be turned of by using --full-diff.

It would be very helpful if you described the problem. "Try it yourself"
is not the answer. In particular, what does "correctly" mean in this
context? Isn't the subject more like "gitk: handle --full-diff" in the
sense that it wasn't handled at all previously and it did what it happened
to do?

The patch breaks gitk's builtin "limit to listed paths" option - with the
patch, the option is only honored at startup, but not when it is changed
during the session when --full-diff is not given. Don't know what should
happen if it is given.

-- Hannes