threads / patch / 31944
Subject: [PATCH 0/2] gitk: handle --full-diff corectly
Felipe ContrerasBasically $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
Felipe Contreras[PATCH 1/2] gitk: simplify file filtering
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
Johannes SixtRe: [PATCH 1/2] gitk: simplify file filtering
Am 10/25/2012 19:23, schrieb Felipe Contreras:
> git diff is perfectly able to do this with '-- files', no need for > manual filtering.
It would have been very useful if you summarized what the code did before this change.
As for the change itself, it seems to make sense to use git diff's path limiting instead of some home-grown method.
-- Hannes
Felipe Contreras[PATCH 2/2] gitk: handle --full-diff correctly
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
Johannes SixtRe: [PATCH 2/2] gitk: handle --full-diff correctly
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