[PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.

[PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.

[Yann Dirson]

This allows to display a set of refs, when the refs in the set may
themselves change between two refresh operations (eg. the set of
patches in a patch stack), and is expected to be called from other
porcelain suites.

The command is expected to generate a list of commits, which will be
appended to the commits litterally passed on the command-line.  That
command is handled similarly to the litteral refs, and has its own
field in the saved view, and an edit field in the view editor.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

This is a rebase to current git master.  It happens that I had to
revert 2 code simplifications you did under "Improve handling of --
and ambiguous arguments", which my patch happens to rely on in its
current form.

 Documentation/gitk.txt |    7 +++++
 gitk                   |   65 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 59 insertions(+), 13 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index e9f82b9..71ed8ba 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -41,6 +41,13 @@ frequently used options.
 
 	Show all branches.
 
+--argscmd=<command>::
+	Command to be run each time gitk has to determine the list of
+	<revs> to show.  The command is expected to print on its
+	standard output a list of additional revs to be shown.  Use
+	this instead of explicitely specifying <revs> if the set of
+	commits to show may vary between refreshs.
+
 <revs>::
 
 	Limit the revisions to show. This can be either a single revision
diff --git a/gitk b/gitk
index f74ce51..264500f 100755
--- a/gitk
+++ b/gitk
@@ -82,18 +82,29 @@ proc dorunq {} {
 proc start_rev_list {view} {
     global startmsecs
     global commfd leftover tclencoding datemode
-    global viewargs viewfiles commitidx
+    global viewargs viewargscmd viewfiles commitidx
     global lookingforhead showlocalchanges
 
     set startmsecs [clock clicks -milliseconds]
     set commitidx($view) 0
+    set args $viewargs($view)
+    if {$viewargscmd($view) ne "None"} {
+	if {[catch {
+	    set fd [open [concat | $viewargscmd($view)] r]
+	} err]} {
+	    puts stderr "Error executing --argscmd command: $err"
+	    exit 1
+	}
+	set args [concat $args [read $fd 500000]]
+	close $fd
+    }
     set order "--topo-order"
     if {$datemode} {
 	set order "--date-order"
     }
     if {[catch {
 	set fd [open [concat | git log -z --pretty=raw $order --parents \
-			 --boundary $viewargs($view) "--" $viewfiles($view)] r]
+			 --boundary $args "--" $viewfiles($view)] r]
     } err]} {
 	error_popup "Error executing git rev-list: $err"
 	exit 1
@@ -966,7 +977,7 @@ proc savestuff {w} {
     global canv canv2 canv3 ctext cflist mainfont textfont uifont tabstop
     global stuffsaved findmergefiles maxgraphpct
     global maxwidth showneartags showlocalchanges
-    global viewname viewfiles viewargs viewperm nextviewnum
+    global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment
     global colors bgcolor fgcolor diffcolors selectbgcolor
 
@@ -1002,7 +1013,7 @@ proc savestuff {w} {
 	puts -nonewline $f "set permviews {"
 	for {set v 0} {$v < $nextviewnum} {incr v} {
 	    if {$viewperm($v)} {
-		puts $f "{[list $viewname($v) $viewfiles($v) $viewargs($v)]}"
+		puts $f "{[list $viewname($v) $viewfiles($v) $viewargs($v) $viewargscmd($v)]}"
 	    }
 	}
 	puts $f "}"
@@ -1597,7 +1608,7 @@ proc shellsplit {str} {
 
 proc newview {ishighlight} {
     global nextviewnum newviewname newviewperm uifont newishighlight
-    global newviewargs revtreeargs
+    global newviewargs revtreeargs viewargscmd newviewargscmd curview
 
     set newishighlight $ishighlight
     set top .gitkview
@@ -1608,13 +1619,14 @@ proc newview {ishighlight} {
     set newviewname($nextviewnum) "View $nextviewnum"
     set newviewperm($nextviewnum) 0
     set newviewargs($nextviewnum) [shellarglist $revtreeargs]
+    set newviewargscmd($nextviewnum) [shellarglist $viewargscmd($curview)]
     vieweditor $top $nextviewnum "Gitk view definition"
 }
 
 proc editview {} {
     global curview
     global viewname viewperm newviewname newviewperm
-    global viewargs newviewargs
+    global viewargs newviewargs viewargscmd newviewargscmd
 
     set top .gitkvedit-$curview
     if {[winfo exists $top]} {
@@ -1624,6 +1636,7 @@ proc editview {} {
     set newviewname($curview) $viewname($curview)
     set newviewperm($curview) $viewperm($curview)
     set newviewargs($curview) [shellarglist $viewargs($curview)]
+    set newviewargscmd($curview) [shellarglist $viewargscmd($curview)]
     vieweditor $top $curview "Gitk: edit view $viewname($curview)"
 }
 
@@ -1644,7 +1657,15 @@ proc vieweditor {top n title} {
     grid $top.al - -sticky w -pady 5
     entry $top.args -width 50 -textvariable newviewargs($n) \
 	-background white -font $uifont
+
     grid $top.args - -sticky ew -padx 5
+    message $top.ac -aspect 1000 -font $uifont \
+	-text "Command to generate more commits to include:"
+    grid $top.ac - -sticky w -pady 5
+    entry $top.argscmd -width 50 -textvariable newviewargscmd($n) \
+	-background white -font $uifont
+
+    grid $top.argscmd - -sticky ew -padx 5
     message $top.l -aspect 1000 -font $uifont \
 	-text "Enter files and directories to include, one per line:"
     grid $top.l - -sticky w
@@ -1690,7 +1711,7 @@ proc allviewmenus {n op args} {
 proc newviewok {top n} {
     global nextviewnum newviewperm newviewname newishighlight
     global viewname viewfiles viewperm selectedview curview
-    global viewargs newviewargs viewhlmenu
+    global viewargs newviewargs viewargscmd newviewargscmd viewhlmenu
 
     if {[catch {
 	set newargs [shellsplit $newviewargs($n)]
@@ -1700,6 +1721,14 @@ proc newviewok {top n} {
 	focus $top
 	return
     }
+    if {[catch {
+	set newargscmd [shellsplit $newviewargscmd($n)]
+    } err]} {
+	error_popup "Error in commit selection command: $err"
+	wm raise $top
+	focus $top
+	return
+    }
     set files {}
     foreach f [split [$top.t get 0.0 end] "\n"] {
 	set ft [string trim $f]
@@ -1714,6 +1743,7 @@ proc newviewok {top n} {
 	set viewperm($n) $newviewperm($n)
 	set viewfiles($n) $files
 	set viewargs($n) $newargs
+	set viewargscmd($n) $newargscmd
 	addviewmenu $n
 	if {!$newishighlight} {
 	    run showview $n
@@ -1730,9 +1760,11 @@ proc newviewok {top n} {
 	    doviewmenu $viewhlmenu 1 [list addvhighlight $n] \
 		entryconf [list -label $viewname($n) -value $viewname($n)]
 	}
-	if {$files ne $viewfiles($n) || $newargs ne $viewargs($n)} {
+	if {$files ne $viewfiles($n) || $newargs ne $viewargs($n) || \
+		$newargscmd ne $viewargscmd($n)} {
 	    set viewfiles($n) $files
 	    set viewargs($n) $newargs
+	    set viewargscmd($n) $newargscmd
 	    if {$curview == $n} {
 		run updatecommits
 	    }
@@ -7584,14 +7616,18 @@ if {![file isdirectory $gitdir]} {
 set revtreeargs {}
 set cmdline_files {}
 set i 0
+set revtreeargscmd None
 foreach arg $argv {
-    switch -- $arg {
-	"" { }
-	"-d" { set datemode 1 }
-	"--" {
+    switch -regexp -- $arg {
+	"^$" { }
+	"^-d" { set datemode 1 }
+	"^--$" {
 	    set cmdline_files [lrange $argv [expr {$i + 1}] end]
 	    break
 	}
+	"^--argscmd=" {
+	    regexp {^--argscmd=(.*)} $arg match revtreeargscmd
+	}
 	default {
 	    lappend revtreeargs $arg
 	}
@@ -7653,6 +7689,7 @@ set selectedhlview None
 set viewfiles(0) {}
 set viewperm(0) 0
 set viewargs(0) {}
+set viewargscmd(0) None
 
 set cmdlineok 0
 set stopped 0
@@ -7669,7 +7706,7 @@ tkwait visibility .
 wm title . "[file tail $argv0]: [file tail [pwd]]"
 readrefs
 
-if {$cmdline_files ne {} || $revtreeargs ne {}} {
+if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {
     # create a view for the files/dirs specified on the command line
     set curview 1
     set selectedview 1
@@ -7677,6 +7714,7 @@ if {$cmdline_files ne {} || $revtreeargs ne {}} {
     set viewname(1) "Command line"
     set viewfiles(1) $cmdline_files
     set viewargs(1) $revtreeargs
+    set viewargscmd(1) $revtreeargscmd
     set viewperm(1) 0
     addviewmenu 1
     .bar.view entryconf Edit* -state normal
@@ -7690,6 +7728,7 @@ if {[info exists permviews]} {
 	set viewname($n) [lindex $v 0]
 	set viewfiles($n) [lindex $v 1]
 	set viewargs($n) [lindex $v 2]
+	set viewargscmd($n) [lindex $v 3]
 	set viewperm($n) 1
 	addviewmenu $n
     }

[PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.

[Yann Dirson]

This allows to display a set of refs, when the refs in the set may
themselves change between two refresh operations (eg. the set of
patches in a patch stack), and is expected to be called from other
porcelain suites.

The command is expected to generate a list of commits, which will be
appended to the commits litterally passed on the command-line.  That
command is handled similarly to the litteral refs, and has its own
field in the saved view, and an edit field in the view editor.

Signed-off-by: Yann Dirson <ydirson@altern.org>
---

This version is the same as v2, just rebased on current git master.  Just let me know
if there are any issues left - notably with the concept of allowing both plain args and
argcmds.

 Documentation/gitk.txt |    7 +++++
 gitk                   |   67 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 60 insertions(+), 14 deletions(-)

diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt
index 8dbfb0d..c3a98d4 100644
--- a/Documentation/gitk.txt
+++ b/Documentation/gitk.txt
@@ -41,6 +41,13 @@ frequently used options.
 
 	Show all branches.
 
+--argscmd=<command>::
+	Command to be run each time gitk has to determine the list of
+	<revs> to show.  The command is expected to print on its
+	standard output a list of additional revs to be shown.  Use
+	this instead of explicitely specifying <revs> if the set of
+	commits to show may vary between refreshs.
+
 <revs>::
 
 	Limit the revisions to show. This can be either a single revision
diff --git a/gitk b/gitk
index 1da0b0a..5e6af40 100755
--- a/gitk
+++ b/gitk
@@ -82,7 +82,7 @@ proc dorunq {} {
 proc start_rev_list {view} {
     global startmsecs
     global commfd leftover tclencoding datemode
-    global viewargs viewfiles commitidx viewcomplete vnextroot
+    global viewargs viewargscmd viewfiles commitidx viewcomplete vnextroot
     global showlocalchanges commitinterest mainheadid
     global progressdirn progresscoords proglastnc curview
 
@@ -90,13 +90,24 @@ proc start_rev_list {view} {
     set commitidx($view) 0
     set viewcomplete($view) 0
     set vnextroot($view) 0
+    set args $viewargs($view)
+    if {$viewargscmd($view) ne "None"} {
+	if {[catch {
+	    set fd [open [concat | $viewargscmd($view)] r]
+	} err]} {
+	    puts stderr "Error executing --argscmd command: $err"
+	    exit 1
+	}
+	set args [concat $args [read $fd 500000]]
+	close $fd
+    }
     set order "--topo-order"
     if {$datemode} {
 	set order "--date-order"
     }
     if {[catch {
 	set fd [open [concat | git log --no-color -z --pretty=raw $order --parents \
-			 --boundary $viewargs($view) "--" $viewfiles($view)] r]
+			 --boundary $args "--" $viewfiles($view)] r]
     } err]} {
 	error_popup "Error executing git rev-list: $err"
 	exit 1
@@ -1154,7 +1165,7 @@ proc savestuff {w} {
     global canv canv2 canv3 mainfont textfont uifont tabstop
     global stuffsaved findmergefiles maxgraphpct
     global maxwidth showneartags showlocalchanges
-    global viewname viewfiles viewargs viewperm nextviewnum
+    global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment datetimeformat limitdiffs
     global colors bgcolor fgcolor diffcolors diffcontext selectbgcolor
 
@@ -1193,7 +1204,7 @@ proc savestuff {w} {
 	puts -nonewline $f "set permviews {"
 	for {set v 0} {$v < $nextviewnum} {incr v} {
 	    if {$viewperm($v)} {
-		puts $f "{[list $viewname($v) $viewfiles($v) $viewargs($v)]}"
+		puts $f "{[list $viewname($v) $viewfiles($v) $viewargs($v) $viewargscmd($v)]}"
 	    }
 	}
 	puts $f "}"
@@ -1849,7 +1860,7 @@ proc shellsplit {str} {
 
 proc newview {ishighlight} {
     global nextviewnum newviewname newviewperm uifont newishighlight
-    global newviewargs revtreeargs
+    global newviewargs revtreeargs viewargscmd newviewargscmd curview
 
     set newishighlight $ishighlight
     set top .gitkview
@@ -1860,13 +1871,14 @@ proc newview {ishighlight} {
     set newviewname($nextviewnum) "View $nextviewnum"
     set newviewperm($nextviewnum) 0
     set newviewargs($nextviewnum) [shellarglist $revtreeargs]
+    set newviewargscmd($nextviewnum) [shellarglist $viewargscmd($curview)]
     vieweditor $top $nextviewnum "Gitk view definition"
 }
 
 proc editview {} {
     global curview
     global viewname viewperm newviewname newviewperm
-    global viewargs newviewargs
+    global viewargs newviewargs viewargscmd newviewargscmd
 
     set top .gitkvedit-$curview
     if {[winfo exists $top]} {
@@ -1876,6 +1888,7 @@ proc editview {} {
     set newviewname($curview) $viewname($curview)
     set newviewperm($curview) $viewperm($curview)
     set newviewargs($curview) [shellarglist $viewargs($curview)]
+    set newviewargscmd($curview) [shellarglist $viewargscmd($curview)]
     vieweditor $top $curview "Gitk: edit view $viewname($curview)"
 }
 
@@ -1896,7 +1909,15 @@ proc vieweditor {top n title} {
     grid $top.al - -sticky w -pady 5
     entry $top.args -width 50 -textvariable newviewargs($n) \
 	-background white -font uifont
+
     grid $top.args - -sticky ew -padx 5
+    message $top.ac -aspect 1000 -font uifont \
+	-text "Command to generate more commits to include:"
+    grid $top.ac - -sticky w -pady 5
+    entry $top.argscmd -width 50 -textvariable newviewargscmd($n) \
+	-background white -font uifont
+
+    grid $top.argscmd - -sticky ew -padx 5
     message $top.l -aspect 1000 -font uifont \
 	-text "Enter files and directories to include, one per line:"
     grid $top.l - -sticky w
@@ -1942,7 +1963,7 @@ proc allviewmenus {n op args} {
 proc newviewok {top n} {
     global nextviewnum newviewperm newviewname newishighlight
     global viewname viewfiles viewperm selectedview curview
-    global viewargs newviewargs viewhlmenu
+    global viewargs newviewargs viewargscmd newviewargscmd viewhlmenu
 
     if {[catch {
 	set newargs [shellsplit $newviewargs($n)]
@@ -1952,6 +1973,14 @@ proc newviewok {top n} {
 	focus $top
 	return
     }
+    if {[catch {
+	set newargscmd [shellsplit $newviewargscmd($n)]
+    } err]} {
+	error_popup "Error in commit selection command: $err"
+	wm raise $top
+	focus $top
+	return
+    }
     set files {}
     foreach f [split [$top.t get 0.0 end] "\n"] {
 	set ft [string trim $f]
@@ -1966,6 +1995,7 @@ proc newviewok {top n} {
 	set viewperm($n) $newviewperm($n)
 	set viewfiles($n) $files
 	set viewargs($n) $newargs
+	set viewargscmd($n) $newargscmd
 	addviewmenu $n
 	if {!$newishighlight} {
 	    run showview $n
@@ -1982,9 +2012,11 @@ proc newviewok {top n} {
 	    # doviewmenu $viewhlmenu 1 [list addvhighlight $n] \
 		# entryconf [list -label $viewname($n) -value $viewname($n)]
 	}
-	if {$files ne $viewfiles($n) || $newargs ne $viewargs($n)} {
+	if {$files ne $viewfiles($n) || $newargs ne $viewargs($n) || \
+		$newargscmd ne $viewargscmd($n)} {
 	    set viewfiles($n) $files
 	    set viewargs($n) $newargs
+	    set viewargscmd($n) $newargscmd
 	    if {$curview == $n} {
 		run updatecommits
 	    }
@@ -8505,18 +8537,22 @@ set mergeonly 0
 set revtreeargs {}
 set cmdline_files {}
 set i 0
+set revtreeargscmd None
 foreach arg $argv {
-    switch -- $arg {
-	"" { }
-	"-d" { set datemode 1 }
-	"--merge" {
+    switch -regexp -- $arg {
+	"^$" { }
+	"^-d$" { set datemode 1 }
+	"^--merge$" {
 	    set mergeonly 1
 	    lappend revtreeargs $arg
 	}
-	"--" {
+	"^--$" {
 	    set cmdline_files [lrange $argv [expr {$i + 1}] end]
 	    break
 	}
+	"^--argscmd=" {
+	    regexp {^--argscmd=(.*)} $arg match revtreeargscmd
+	}
 	default {
 	    lappend revtreeargs $arg
 	}
@@ -8618,6 +8654,7 @@ set highlight_files {}
 set viewfiles(0) {}
 set viewperm(0) 0
 set viewargs(0) {}
+set viewargscmd(0) None
 
 set cmdlineok 0
 set stopped 0
@@ -8633,7 +8670,7 @@ tkwait visibility .
 wm title . "[file tail $argv0]: [file tail [pwd]]"
 readrefs
 
-if {$cmdline_files ne {} || $revtreeargs ne {}} {
+if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {
     # create a view for the files/dirs specified on the command line
     set curview 1
     set selectedview 1
@@ -8641,6 +8678,7 @@ if {$cmdline_files ne {} || $revtreeargs ne {}} {
     set viewname(1) "Command line"
     set viewfiles(1) $cmdline_files
     set viewargs(1) $revtreeargs
+    set viewargscmd(1) $revtreeargscmd
     set viewperm(1) 0
     addviewmenu 1
     .bar.view entryconf Edit* -state normal
@@ -8654,6 +8692,7 @@ if {[info exists permviews]} {
 	set viewname($n) [lindex $v 0]
 	set viewfiles($n) [lindex $v 1]
 	set viewargs($n) [lindex $v 2]
+	set viewargscmd($n) [lindex $v 3]
 	set viewperm($n) 1
 	addviewmenu $n
     }

Re: [PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.

[Paul Mackerras]

Yann Dirson writes:

> This allows to display a set of refs, when the refs in the set may
> themselves change between two refresh operations (eg. the set of
> patches in a patch stack), and is expected to be called from other
> porcelain suites.
> 
> The command is expected to generate a list of commits, which will be
> appended to the commits litterally passed on the command-line.  That
> command is handled similarly to the litteral refs, and has its own
> field in the saved view, and an edit field in the view editor.

The idea is fine, but I have some comments on the implementation:

> +--argscmd=<command>::
> +	Command to be run each time gitk has to determine the list of
> +	<revs> to show.  The command is expected to print on its
> +	standard output a list of additional revs to be shown.  Use

, one per line

> +	this instead of explicitely specifying <revs> if the set of

explicitly

> +	commits to show may vary between refreshs.

refreshes

> +    set args $viewargs($view)
> +    if {$viewargscmd($view) ne "None"} {
> +	if {[catch {
> +	    set fd [open [concat | $viewargscmd($view)] r]
> +	} err]} {
> +	    puts stderr "Error executing --argscmd command: $err"
> +	    exit 1
> +	}
> +	set args [concat $args [read $fd 500000]]

I don't think you necessarily want to limit the number of characters
read to 500000, do you?

What this will do is interpret the output of the program according to
Tcl list syntax.  I think it would be better to use [split $str "\n"]
to split the program's output at the newlines.  Also, you could
combine the open, read and close into a single exec call.  Thirdly,
use error_popup rather than just writing to stderr.

I wonder if you should be invoking a shell to interpret the command.
As you have it at the moment, the string that the user puts after
--argscmd= is treated as a Tcl list, whereas the string they put into
the entry field when creating a new view is split with shellsplit,
which implements shell-style quoting.  I think we should at least be
consistent here, and possibly just leave it as a string and pass it to
sh -c.

> +set revtreeargscmd None

Why the string "None" rather than the empty string?  Is this a
python-ism that has crept in?

>  foreach arg $argv {
> -    switch -- $arg {
> -	"" { }
> -	"-d" { set datemode 1 }
> -	"--merge" {
> +    switch -regexp -- $arg {

Hmmm, it'd be simpler to use switch -glob here, I think.

> +	"^$" { }
> +	"^-d$" { set datemode 1 }
> +	"^--merge$" {
>  	    set mergeonly 1
>  	    lappend revtreeargs $arg
>  	}
> -	"--" {
> +	"^--$" {
>  	    set cmdline_files [lrange $argv [expr {$i + 1}] end]
>  	    break
>  	}
> +	"^--argscmd=" {
> +	    regexp {^--argscmd=(.*)} $arg match revtreeargscmd

set revtreeargscmd [string range $arg 10 end]

Paul.

Re: [PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.

[Yann Dirson]

Hi Paul,

I hope I did not introduce other problems while trying to adress those.

On Sat, Nov 17, 2007 at 10:44:00PM +1100, Paul Mackerras wrote:
> What this will do is interpret the output of the program according to
> Tcl list syntax.

I'm not sure of why this interpretation is done, so it may well be that
the updated patch would behave the same.

> use error_popup rather than just writing to stderr.

While testing that, I realized that the current behaviour is not very
friendly.  While it is OK to exit on error here, when the command has
been entered on command-line, it would be rude to exit when the user
makes a typo when editing the view.  It would be more friendly to get
back to the "edit view" dialog instead; I'm not sure we can already
tell in which of the 2 situations we are - would you conisider adding
a global flag for tracking this to be an adequate solution ?

> > +set revtreeargscmd None
> 
> Why the string "None" rather than the empty string?  Is this a
> python-ism that has crept in?

Note completely - since there were already occurences of None in gitk, I
assumed it was a special value in Tcl.  Should I have searched in more
details, I would have noticed the string comparisons.

Best regards,
-- 
Yann