{"thread":{"id":"41739","subject":"[PATCH v2 1/2] gitk: alter the ordering for the \"Tags and heads\" view","startedAt":"2016-03-18T18:51:35Z","lastAt":"2016-03-19T06:55:28Z","messageCount":5,"participants":["Michael Rappazzo","Paul Mackerras"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"281169","messageId":"1458327097-92126-1-git-send-email-rappazzo@gmail.com","threadId":"41739","inReplyTo":null,"subject":"[PATCH v2 0/2] gitk: changes for the \"Tags and heads\" view","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-03-18T18:51:35Z","receivedAt":"2016-03-18T18:51:35Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"I found a small bug in the v1 of this patch[1], when a tracked upstream\nbranch is no longer present.  The changes are only in the first commit.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/288544\n\nMichael Rappazzo (2):\n  gitk: alter the ordering for the \"Tags and heads\" view\n  gitk: add an option to enable sorting the \"Tags and heads\" view by ref\n    type\n\n gitk | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------\n 1 file changed, 52 insertions(+), 7 deletions(-)\n\n-- \n2.7.4\n"},{"id":"281168","messageId":"1458327097-92126-2-git-send-email-rappazzo@gmail.com","threadId":"41739","inReplyTo":"1458327097-92126-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v2 1/2] gitk: alter the ordering for the \"Tags and heads\" view","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-03-18T18:51:36Z","receivedAt":"2016-03-18T18:51:36Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"In the \"Tags and heads\" view, the list of refs is globally sorted.\nBecause of this, the list of local refs (heads) can be interrupted by the\nlist of remote refs.  This change re-orders the view to be: local refs,\nremote refs tracked by local refs, remote refs, tags, and then other refs.\n\nSigned-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n gitk | 48 ++++++++++++++++++++++++++++++++++++++++++------\n 1 file changed, 42 insertions(+), 6 deletions(-)\n\ndiff --git a/gitk b/gitk\nindex 5f1255c..d6d3136 100755\n--- a/gitk\n+++ b/gitk\n@@ -9933,35 +9933,71 @@ proc refill_reflist {} {\n     global curview\n \n     if {![info exists showrefstop] || ![winfo exists $showrefstop]} return\n-    set refs {}\n+    set localrefs {}\n+    set remoterefs {}\n+    set locally_tracked_remote_refs {}\n+    set tagrefs {}\n+    set otherrefs {}\n     foreach n [array names headids] {\n-\tif {[string match $reflistfilter $n]} {\n+\tif {![string match \"remotes/*\" $n] && [string match $reflistfilter $n]} {\n \t    if {[commitinview $headids($n) $curview]} {\n-\t\tlappend refs [list $n H]\n+\t\tlappend localrefs [list $n H]\n+\t\tcatch {set remote_name [exec git config --get branch.$n.remote]}\n+\t\tif {$remote_name ne \"\"} {\n+\t\t    catch {set remote_ref [exec git config --get branch.$n.merge]}\n+\t\t    set remote_ref [string map {\"refs/heads/\" \"\"} $remote_ref]\n+\t\t    set locally_tracked_remote_ref \"remotes/$remote_name/$remote_ref\"\n+\t\t    set upstream_exists \"\"\n+\t\t    catch {set upstream_exists [exec git rev-parse --verify $locally_tracked_remote_ref]}\n+\t\t    if {$upstream_exists ne \"\"} {\n+\t\t\tif {[lsearch $locally_tracked_remote_refs [list $locally_tracked_remote_ref H]] < 0} {\n+\t\t\t    lappend locally_tracked_remote_refs [list $locally_tracked_remote_ref H]\n+\t\t\t}\n+\t\t    }\n+\t\t}\n \t    } else {\n \t\tinterestedin $headids($n) {run refill_reflist}\n \t    }\n \t}\n     }\n+    set locally_tracked_remote_refs [lsort -index 0 $locally_tracked_remote_refs]\n+    set localrefs [lsort -index 0 $localrefs]\n+\n+    foreach n [array names headids] {\n+\tif {[string match \"remotes/*\" $n] && [string match $reflistfilter $n]} {\n+\t    if {[commitinview $headids($n) $curview]} {\n+\t\tif {[lsearch $locally_tracked_remote_refs [list $n H]] < 0} {\n+\t\t    lappend remoterefs [list $n H]\n+\t\t}\n+\t    } else {\n+\t\tinterestedin $headids($n) {run refill_reflist}\n+\t    }\n+\t}\n+    }\n+    set remoterefs [lsort -index 0 $remoterefs]\n+\n     foreach n [array names tagids] {\n \tif {[string match $reflistfilter $n]} {\n \t    if {[commitinview $tagids($n) $curview]} {\n-\t\tlappend refs [list $n T]\n+\t\tlappend tagrefs [list $n T]\n \t    } else {\n \t\tinterestedin $tagids($n) {run refill_reflist}\n \t    }\n \t}\n     }\n+    set tagrefs [lsort -index 0 $tagrefs]\n+\n     foreach n [array names otherrefids] {\n \tif {[string match $reflistfilter $n]} {\n \t    if {[commitinview $otherrefids($n) $curview]} {\n-\t\tlappend refs [list $n o]\n+\t\tlappend otherrefs [list \"$n\" o]\n \t    } else {\n \t\tinterestedin $otherrefids($n) {run refill_reflist}\n \t    }\n \t}\n     }\n-    set refs [lsort -index 0 $refs]\n+    set otherrefs [lsort -index 0 $otherrefs]\n+    lappend refs {*}$localrefs {*}$locally_tracked_remote_refs {*}$remoterefs {*}$tagrefs {*}$otherrefs\n     if {$refs eq $reflist} return\n \n     # Update the contents of $showrefstop.list according to the\n-- \n2.7.4\n"},{"id":"281170","messageId":"1458327097-92126-3-git-send-email-rappazzo@gmail.com","threadId":"41739","inReplyTo":"1458327097-92126-1-git-send-email-rappazzo@gmail.com","subject":"[PATCH v2 2/2] gitk: add an option to enable sorting the \"Tags and heads\" view by ref type","fromName":"Michael Rappazzo","fromEmail":"rappazzo@gmail.com","sentAt":"2016-03-18T18:51:37Z","receivedAt":"2016-03-18T18:51:37Z","isPatch":true,"sender":{"key":"rappazzo@gmail.com","avatar":"https://avatars.githubusercontent.com/u/525287?v=4"},"body":"Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n---\n gitk | 11 ++++++++++-\n 1 file changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/gitk b/gitk\nindex d6d3136..6d376e3 100755\n--- a/gitk\n+++ b/gitk\n@@ -158,6 +158,7 @@ proc parseviewargs {n arglist} {\n     global vdatemode vmergeonly vflags vdflags vrevs vfiltered vorigargs env\n     global vinlinediff\n     global worddiff git_version\n+    global sort_refs_by_type\n \n     set vdatemode($n) 0\n     set vmergeonly($n) 0\n@@ -170,6 +171,7 @@ proc parseviewargs {n arglist} {\n     set allknown 1\n     set filtered 0\n     set i -1\n+    set sort_refs_by_type \"0\"\n     foreach arg $arglist {\n \tincr i\n \tif {$nextisval} {\n@@ -261,6 +263,9 @@ proc parseviewargs {n arglist} {\n \t    \"--no-replace-objects\" {\n \t\tset env(GIT_NO_REPLACE_OBJECTS) \"1\"\n \t    }\n+\t    \"--sort-refs-by-type\" {\n+\t\tset sort_refs_by_type \"1\"\n+\t    }\n \t    \"-*\" {\n \t\t# Other flag arguments including -<n>\n \t\tif {[string is digit -strict [string range $arg 1 end]]} {\n@@ -9929,7 +9934,7 @@ proc reflistfilter_change {n1 n2 op} {\n }\n \n proc refill_reflist {} {\n-    global reflist reflistfilter showrefstop headids tagids otherrefids\n+    global reflist reflistfilter showrefstop headids tagids otherrefids sort_refs_by_type\n     global curview\n \n     if {![info exists showrefstop] || ![winfo exists $showrefstop]} return\n@@ -9998,6 +10003,10 @@ proc refill_reflist {} {\n     }\n     set otherrefs [lsort -index 0 $otherrefs]\n     lappend refs {*}$localrefs {*}$locally_tracked_remote_refs {*}$remoterefs {*}$tagrefs {*}$otherrefs\n+    if {$sort_refs_by_type ne \"1\"} {\n+       set refs [lsort -index 0 $refs]\n+    }\n+\n     if {$refs eq $reflist} return\n \n     # Update the contents of $showrefstop.list according to the\n-- \n2.7.4\n"},{"id":"281213","messageId":"20160319065347.GC27126@fergus.ozlabs.ibm.com","threadId":"41739","inReplyTo":"1458327097-92126-2-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2 1/2] gitk: alter the ordering for the \"Tags and heads\" view","fromName":"Paul Mackerras","fromEmail":"paulus@ozlabs.org","sentAt":"2016-03-19T06:53:47Z","receivedAt":"2016-03-19T06:53:47Z","isPatch":true,"sender":{"key":"paulus@ozlabs.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Fri, Mar 18, 2016 at 02:51:36PM -0400, Michael Rappazzo wrote:\n> In the \"Tags and heads\" view, the list of refs is globally sorted.\n> Because of this, the list of local refs (heads) can be interrupted by the\n> list of remote refs.  This change re-orders the view to be: local refs,\n> remote refs tracked by local refs, remote refs, tags, and then other refs.\n\nThis seems like a nice idea, and could lead on to having a section that\ncan be opened and closed for each of these categories.  However, I\nhave some comments on the implementation:\n\n> @@ -9933,35 +9933,71 @@ proc refill_reflist {} {\n>      global curview\n>  \n>      if {![info exists showrefstop] || ![winfo exists $showrefstop]} return\n> -    set refs {}\n> +    set localrefs {}\n> +    set remoterefs {}\n> +    set locally_tracked_remote_refs {}\n> +    set tagrefs {}\n> +    set otherrefs {}\n>      foreach n [array names headids] {\n> -\tif {[string match $reflistfilter $n]} {\n> +\tif {![string match \"remotes/*\" $n] && [string match $reflistfilter $n]} {\n>  \t    if {[commitinview $headids($n) $curview]} {\n> -\t\tlappend refs [list $n H]\n> +\t\tlappend localrefs [list $n H]\n> +\t\tcatch {set remote_name [exec git config --get branch.$n.remote]}\n> +\t\tif {$remote_name ne \"\"} {\n> +\t\t    catch {set remote_ref [exec git config --get branch.$n.merge]}\n> +\t\t    set remote_ref [string map {\"refs/heads/\" \"\"} $remote_ref]\n> +\t\t    set locally_tracked_remote_ref \"remotes/$remote_name/$remote_ref\"\n> +\t\t    set upstream_exists \"\"\n> +\t\t    catch {set upstream_exists [exec git rev-parse --verify $locally_tracked_remote_ref]}\n> +\t\t    if {$upstream_exists ne \"\"} {\n> +\t\t\tif {[lsearch $locally_tracked_remote_refs [list $locally_tracked_remote_ref H]] < 0} {\n> +\t\t\t    lappend locally_tracked_remote_refs [list $locally_tracked_remote_ref H]\n> +\t\t\t}\n> +\t\t    }\n> +\t\t}\n\nI'm worried about the number of external git invocations we're\npotentially doing here, and how long that would take when you have a\nlot of heads.  Can we cache the results perhaps?  Or is there a way to\ndo a single git command and get a list of tracking branches with their\nremotes etc.?\n\nAlso, the default for lsearch is glob-style matching.  It's unlikely\nthat ref names would have any of *?[\\ in them, but AFAIK it's not\nimpossible.  You probably want -exact in there.\n\nFurther, the kind of thing you are using lsearch for can often be done\nmore efficiently using an array (which becomes essentially a hash\ntable underneath).\n\nPaul.\n"},{"id":"281214","messageId":"20160319065528.GD27126@fergus.ozlabs.ibm.com","threadId":"41739","inReplyTo":"1458327097-92126-3-git-send-email-rappazzo@gmail.com","subject":"Re: [PATCH v2 2/2] gitk: add an option to enable sorting the \"Tags and heads\" view by ref type","fromName":"Paul Mackerras","fromEmail":"paulus@ozlabs.org","sentAt":"2016-03-19T06:55:28Z","receivedAt":"2016-03-19T06:55:28Z","isPatch":true,"sender":{"key":"paulus@ozlabs.org","avatar":"https://avatars.githubusercontent.com/u/1606439?v=4"},"body":"On Fri, Mar 18, 2016 at 02:51:37PM -0400, Michael Rappazzo wrote:\n> Signed-off-by: Michael Rappazzo <rappazzo@gmail.com>\n\nThis looks to me like the kind of thing that could have a checkbox in\nthe Preferences window and get saved on disk along with other\npreferences, rather than only being available via a command-line\noption.\n\nPaul.\n"}]}