{"thread":{"id":"10601","subject":"[PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.","startedAt":"2007-11-02T22:24:50Z","lastAt":"2007-11-17T19:56:59Z","messageCount":3,"participants":["Yann Dirson","Paul Mackerras"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"58048","messageId":"20071102222436.23191.91785.stgit@gandelf.nowhere.earth","threadId":"10601","inReplyTo":null,"subject":"[PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-11-02T22:24:50Z","receivedAt":"2007-11-02T22:24:50Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"This allows to display a set of refs, when the refs in the set may\nthemselves change between two refresh operations (eg. the set of\npatches in a patch stack), and is expected to be called from other\nporcelain suites.\n\nThe command is expected to generate a list of commits, which will be\nappended to the commits litterally passed on the command-line.  That\ncommand is handled similarly to the litteral refs, and has its own\nfield in the saved view, and an edit field in the view editor.\n\nSigned-off-by: Yann Dirson <ydirson@altern.org>\n---\n\nThis version is the same as v2, just rebased on current git master.  Just let me know\nif there are any issues left - notably with the concept of allowing both plain args and\nargcmds.\n\n Documentation/gitk.txt |    7 +++++\n gitk                   |   67 ++++++++++++++++++++++++++++++++++++++----------\n 2 files changed, 60 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/gitk.txt b/Documentation/gitk.txt\nindex 8dbfb0d..c3a98d4 100644\n--- a/Documentation/gitk.txt\n+++ b/Documentation/gitk.txt\n@@ -41,6 +41,13 @@ frequently used options.\n \n \tShow all branches.\n \n+--argscmd=<command>::\n+\tCommand to be run each time gitk has to determine the list of\n+\t<revs> to show.  The command is expected to print on its\n+\tstandard output a list of additional revs to be shown.  Use\n+\tthis instead of explicitely specifying <revs> if the set of\n+\tcommits to show may vary between refreshs.\n+\n <revs>::\n \n \tLimit the revisions to show. This can be either a single revision\ndiff --git a/gitk b/gitk\nindex 1da0b0a..5e6af40 100755\n--- a/gitk\n+++ b/gitk\n@@ -82,7 +82,7 @@ proc dorunq {} {\n proc start_rev_list {view} {\n     global startmsecs\n     global commfd leftover tclencoding datemode\n-    global viewargs viewfiles commitidx viewcomplete vnextroot\n+    global viewargs viewargscmd viewfiles commitidx viewcomplete vnextroot\n     global showlocalchanges commitinterest mainheadid\n     global progressdirn progresscoords proglastnc curview\n \n@@ -90,13 +90,24 @@ proc start_rev_list {view} {\n     set commitidx($view) 0\n     set viewcomplete($view) 0\n     set vnextroot($view) 0\n+    set args $viewargs($view)\n+    if {$viewargscmd($view) ne \"None\"} {\n+\tif {[catch {\n+\t    set fd [open [concat | $viewargscmd($view)] r]\n+\t} err]} {\n+\t    puts stderr \"Error executing --argscmd command: $err\"\n+\t    exit 1\n+\t}\n+\tset args [concat $args [read $fd 500000]]\n+\tclose $fd\n+    }\n     set order \"--topo-order\"\n     if {$datemode} {\n \tset order \"--date-order\"\n     }\n     if {[catch {\n \tset fd [open [concat | git log --no-color -z --pretty=raw $order --parents \\\n-\t\t\t --boundary $viewargs($view) \"--\" $viewfiles($view)] r]\n+\t\t\t --boundary $args \"--\" $viewfiles($view)] r]\n     } err]} {\n \terror_popup \"Error executing git rev-list: $err\"\n \texit 1\n@@ -1154,7 +1165,7 @@ proc savestuff {w} {\n     global canv canv2 canv3 mainfont textfont uifont tabstop\n     global stuffsaved findmergefiles maxgraphpct\n     global maxwidth showneartags showlocalchanges\n-    global viewname viewfiles viewargs viewperm nextviewnum\n+    global viewname viewfiles viewargs viewargscmd viewperm nextviewnum\n     global cmitmode wrapcomment datetimeformat limitdiffs\n     global colors bgcolor fgcolor diffcolors diffcontext selectbgcolor\n \n@@ -1193,7 +1204,7 @@ proc savestuff {w} {\n \tputs -nonewline $f \"set permviews {\"\n \tfor {set v 0} {$v < $nextviewnum} {incr v} {\n \t    if {$viewperm($v)} {\n-\t\tputs $f \"{[list $viewname($v) $viewfiles($v) $viewargs($v)]}\"\n+\t\tputs $f \"{[list $viewname($v) $viewfiles($v) $viewargs($v) $viewargscmd($v)]}\"\n \t    }\n \t}\n \tputs $f \"}\"\n@@ -1849,7 +1860,7 @@ proc shellsplit {str} {\n \n proc newview {ishighlight} {\n     global nextviewnum newviewname newviewperm uifont newishighlight\n-    global newviewargs revtreeargs\n+    global newviewargs revtreeargs viewargscmd newviewargscmd curview\n \n     set newishighlight $ishighlight\n     set top .gitkview\n@@ -1860,13 +1871,14 @@ proc newview {ishighlight} {\n     set newviewname($nextviewnum) \"View $nextviewnum\"\n     set newviewperm($nextviewnum) 0\n     set newviewargs($nextviewnum) [shellarglist $revtreeargs]\n+    set newviewargscmd($nextviewnum) [shellarglist $viewargscmd($curview)]\n     vieweditor $top $nextviewnum \"Gitk view definition\"\n }\n \n proc editview {} {\n     global curview\n     global viewname viewperm newviewname newviewperm\n-    global viewargs newviewargs\n+    global viewargs newviewargs viewargscmd newviewargscmd\n \n     set top .gitkvedit-$curview\n     if {[winfo exists $top]} {\n@@ -1876,6 +1888,7 @@ proc editview {} {\n     set newviewname($curview) $viewname($curview)\n     set newviewperm($curview) $viewperm($curview)\n     set newviewargs($curview) [shellarglist $viewargs($curview)]\n+    set newviewargscmd($curview) [shellarglist $viewargscmd($curview)]\n     vieweditor $top $curview \"Gitk: edit view $viewname($curview)\"\n }\n \n@@ -1896,7 +1909,15 @@ proc vieweditor {top n title} {\n     grid $top.al - -sticky w -pady 5\n     entry $top.args -width 50 -textvariable newviewargs($n) \\\n \t-background white -font uifont\n+\n     grid $top.args - -sticky ew -padx 5\n+    message $top.ac -aspect 1000 -font uifont \\\n+\t-text \"Command to generate more commits to include:\"\n+    grid $top.ac - -sticky w -pady 5\n+    entry $top.argscmd -width 50 -textvariable newviewargscmd($n) \\\n+\t-background white -font uifont\n+\n+    grid $top.argscmd - -sticky ew -padx 5\n     message $top.l -aspect 1000 -font uifont \\\n \t-text \"Enter files and directories to include, one per line:\"\n     grid $top.l - -sticky w\n@@ -1942,7 +1963,7 @@ proc allviewmenus {n op args} {\n proc newviewok {top n} {\n     global nextviewnum newviewperm newviewname newishighlight\n     global viewname viewfiles viewperm selectedview curview\n-    global viewargs newviewargs viewhlmenu\n+    global viewargs newviewargs viewargscmd newviewargscmd viewhlmenu\n \n     if {[catch {\n \tset newargs [shellsplit $newviewargs($n)]\n@@ -1952,6 +1973,14 @@ proc newviewok {top n} {\n \tfocus $top\n \treturn\n     }\n+    if {[catch {\n+\tset newargscmd [shellsplit $newviewargscmd($n)]\n+    } err]} {\n+\terror_popup \"Error in commit selection command: $err\"\n+\twm raise $top\n+\tfocus $top\n+\treturn\n+    }\n     set files {}\n     foreach f [split [$top.t get 0.0 end] \"\\n\"] {\n \tset ft [string trim $f]\n@@ -1966,6 +1995,7 @@ proc newviewok {top n} {\n \tset viewperm($n) $newviewperm($n)\n \tset viewfiles($n) $files\n \tset viewargs($n) $newargs\n+\tset viewargscmd($n) $newargscmd\n \taddviewmenu $n\n \tif {!$newishighlight} {\n \t    run showview $n\n@@ -1982,9 +2012,11 @@ proc newviewok {top n} {\n \t    # doviewmenu $viewhlmenu 1 [list addvhighlight $n] \\\n \t\t# entryconf [list -label $viewname($n) -value $viewname($n)]\n \t}\n-\tif {$files ne $viewfiles($n) || $newargs ne $viewargs($n)} {\n+\tif {$files ne $viewfiles($n) || $newargs ne $viewargs($n) || \\\n+\t\t$newargscmd ne $viewargscmd($n)} {\n \t    set viewfiles($n) $files\n \t    set viewargs($n) $newargs\n+\t    set viewargscmd($n) $newargscmd\n \t    if {$curview == $n} {\n \t\trun updatecommits\n \t    }\n@@ -8505,18 +8537,22 @@ set mergeonly 0\n set revtreeargs {}\n set cmdline_files {}\n set i 0\n+set revtreeargscmd None\n foreach arg $argv {\n-    switch -- $arg {\n-\t\"\" { }\n-\t\"-d\" { set datemode 1 }\n-\t\"--merge\" {\n+    switch -regexp -- $arg {\n+\t\"^$\" { }\n+\t\"^-d$\" { set datemode 1 }\n+\t\"^--merge$\" {\n \t    set mergeonly 1\n \t    lappend revtreeargs $arg\n \t}\n-\t\"--\" {\n+\t\"^--$\" {\n \t    set cmdline_files [lrange $argv [expr {$i + 1}] end]\n \t    break\n \t}\n+\t\"^--argscmd=\" {\n+\t    regexp {^--argscmd=(.*)} $arg match revtreeargscmd\n+\t}\n \tdefault {\n \t    lappend revtreeargs $arg\n \t}\n@@ -8618,6 +8654,7 @@ set highlight_files {}\n set viewfiles(0) {}\n set viewperm(0) 0\n set viewargs(0) {}\n+set viewargscmd(0) None\n \n set cmdlineok 0\n set stopped 0\n@@ -8633,7 +8670,7 @@ tkwait visibility .\n wm title . \"[file tail $argv0]: [file tail [pwd]]\"\n readrefs\n \n-if {$cmdline_files ne {} || $revtreeargs ne {}} {\n+if {$cmdline_files ne {} || $revtreeargs ne {} || $revtreeargscmd ne {}} {\n     # create a view for the files/dirs specified on the command line\n     set curview 1\n     set selectedview 1\n@@ -8641,6 +8678,7 @@ if {$cmdline_files ne {} || $revtreeargs ne {}} {\n     set viewname(1) \"Command line\"\n     set viewfiles(1) $cmdline_files\n     set viewargs(1) $revtreeargs\n+    set viewargscmd(1) $revtreeargscmd\n     set viewperm(1) 0\n     addviewmenu 1\n     .bar.view entryconf Edit* -state normal\n@@ -8654,6 +8692,7 @@ if {[info exists permviews]} {\n \tset viewname($n) [lindex $v 0]\n \tset viewfiles($n) [lindex $v 1]\n \tset viewargs($n) [lindex $v 2]\n+\tset viewargscmd($n) [lindex $v 3]\n \tset viewperm($n) 1\n \taddviewmenu $n\n     }\n"},{"id":"60131","messageId":"18238.54272.198637.788805@cargo.ozlabs.ibm.com","threadId":"10601","inReplyTo":"20071102222436.23191.91785.stgit@gandelf.nowhere.earth","subject":"Re: [PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.","fromName":"Paul Mackerras","fromEmail":"paulus@samba.org","sentAt":"2007-11-17T11:44:00Z","receivedAt":"2007-11-17T11:44:00Z","isPatch":true,"sender":{"key":"paulus@samba.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"Yann Dirson writes:\n\n> This allows to display a set of refs, when the refs in the set may\n> themselves change between two refresh operations (eg. the set of\n> patches in a patch stack), and is expected to be called from other\n> porcelain suites.\n> \n> The command is expected to generate a list of commits, which will be\n> appended to the commits litterally passed on the command-line.  That\n> command is handled similarly to the litteral refs, and has its own\n> field in the saved view, and an edit field in the view editor.\n\nThe idea is fine, but I have some comments on the implementation:\n\n> +--argscmd=<command>::\n> +\tCommand to be run each time gitk has to determine the list of\n> +\t<revs> to show.  The command is expected to print on its\n> +\tstandard output a list of additional revs to be shown.  Use\n\n, one per line\n\n> +\tthis instead of explicitely specifying <revs> if the set of\n\nexplicitly\n\n> +\tcommits to show may vary between refreshs.\n\nrefreshes\n\n> +    set args $viewargs($view)\n> +    if {$viewargscmd($view) ne \"None\"} {\n> +\tif {[catch {\n> +\t    set fd [open [concat | $viewargscmd($view)] r]\n> +\t} err]} {\n> +\t    puts stderr \"Error executing --argscmd command: $err\"\n> +\t    exit 1\n> +\t}\n> +\tset args [concat $args [read $fd 500000]]\n\nI don't think you necessarily want to limit the number of characters\nread to 500000, do you?\n\nWhat this will do is interpret the output of the program according to\nTcl list syntax.  I think it would be better to use [split $str \"\\n\"]\nto split the program's output at the newlines.  Also, you could\ncombine the open, read and close into a single exec call.  Thirdly,\nuse error_popup rather than just writing to stderr.\n\nI wonder if you should be invoking a shell to interpret the command.\nAs you have it at the moment, the string that the user puts after\n--argscmd= is treated as a Tcl list, whereas the string they put into\nthe entry field when creating a new view is split with shellsplit,\nwhich implements shell-style quoting.  I think we should at least be\nconsistent here, and possibly just leave it as a string and pass it to\nsh -c.\n\n> +set revtreeargscmd None\n\nWhy the string \"None\" rather than the empty string?  Is this a\npython-ism that has crept in?\n\n>  foreach arg $argv {\n> -    switch -- $arg {\n> -\t\"\" { }\n> -\t\"-d\" { set datemode 1 }\n> -\t\"--merge\" {\n> +    switch -regexp -- $arg {\n\nHmmm, it'd be simpler to use switch -glob here, I think.\n\n> +\t\"^$\" { }\n> +\t\"^-d$\" { set datemode 1 }\n> +\t\"^--merge$\" {\n>  \t    set mergeonly 1\n>  \t    lappend revtreeargs $arg\n>  \t}\n> -\t\"--\" {\n> +\t\"^--$\" {\n>  \t    set cmdline_files [lrange $argv [expr {$i + 1}] end]\n>  \t    break\n>  \t}\n> +\t\"^--argscmd=\" {\n> +\t    regexp {^--argscmd=(.*)} $arg match revtreeargscmd\n\nset revtreeargscmd [string range $arg 10 end]\n\nPaul.\n"},{"id":"60163","messageId":"20071117195659.GO8855@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"10601","inReplyTo":"18238.54272.198637.788805@cargo.ozlabs.ibm.com","subject":"Re: [PATCH v3] Add to gitk an --argscmd flag to get the list of refs to draw at refresh time.","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-11-17T19:56:59Z","receivedAt":"2007-11-17T19:56:59Z","isPatch":true,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"Hi Paul,\n\nI hope I did not introduce other problems while trying to adress those.\n\nOn Sat, Nov 17, 2007 at 10:44:00PM +1100, Paul Mackerras wrote:\n> What this will do is interpret the output of the program according to\n> Tcl list syntax.\n\nI'm not sure of why this interpretation is done, so it may well be that\nthe updated patch would behave the same.\n\n> use error_popup rather than just writing to stderr.\n\nWhile testing that, I realized that the current behaviour is not very\nfriendly.  While it is OK to exit on error here, when the command has\nbeen entered on command-line, it would be rude to exit when the user\nmakes a typo when editing the view.  It would be more friendly to get\nback to the \"edit view\" dialog instead; I'm not sure we can already\ntell in which of the 2 situations we are - would you conisider adding\na global flag for tracking this to be an adequate solution ?\n\n> > +set revtreeargscmd None\n> \n> Why the string \"None\" rather than the empty string?  Is this a\n> python-ism that has crept in?\n\nNote completely - since there were already occurences of None in gitk, I\nassumed it was a special value in Tcl.  Should I have searched in more\ndetails, I would have noticed the string comparisons.\n\nBest regards,\n-- \nYann\n"}]}