gitk: Fix how remote branch names with / are drawn

Consider this example branch:

remotes/origin/master

gitk displays this branch with different background colors for each part:
"remotes/origin" in orange and "master" in green. The idea is to make it visually easy to read the branch name separately from the remote name.

However this fails when given this example branch:

remotes/origin/foo/bar

gitk displays this branch with "remotes/origin/foo" in orange and "bar" in green. This makes it hard to read the branch name "foo/bar". This is due to an inappropriately greedy regexp. This patch provides a fix so the same branch will now be displayed with "remotes/origin" in orange and "foo/bar" in green.

Signed-off-by David Holmer <[email protected]>

---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

gitk +0 -0

index 805a1c7..ca2392b 100755

Quoted 28 lines · David Holmer

Consider this example branch:

remotes/origin/master

gitk displays this branch with different background colors for each part:
"remotes/origin" in orange and "master" in green. The idea is to make it visually easy to read the branch name separately from the remote name.

However this fails when given this example branch:

remotes/origin/foo/bar

gitk displays this branch with "remotes/origin/foo" in orange and "bar" in green. This makes it hard to read the branch name "foo/bar". This is due to an inappropriately greedy regexp. This patch provides a fix so the same branch will now be displayed with "remotes/origin" in orange and "foo/bar" in green.

Signed-off-by David Holmer <[email protected]>

---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

gitk +0 -0

index 805a1c7..ca2392b 100755

gitk proc drawtags {id x xt y1} {

Quoted 9 lines

gitk +1 -2

@@ -6640,7 +6640,7 @@ proc drawtags {id x xt y1} {
            set xl [expr {$xl - $delta/2}]
            $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
                -width 1 -outline black -fill $col -tags tag.$id
-           if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
+           if {[regexp {^(remotes/[^/]*/|remotes/)} $tag match remoteprefix]} {
                set rwid [font measure mainfont $remoteprefix]
                set xi [expr {$x + 1}]
                set yti [expr {$yt + 1}]
--

This likely fixes the problem for most situations, but doesn't for a remote with a '/' in the name. Yet, I think this is a better state than the present.

Is the regex [^/]*/ more efficient than '.*?/`? Or do you find the former more readable?

I agree that this switches the issue around and that a remote with a '/' in the name would be miss colored in the same way a branch with a '/' in the name is miss colored now. However, I would guess that branches with '/' are MUCH MUCH more common than remotes with '/', so like you say "this is a better state than the present". A "complete" solution would take iterating through the list of remotes and matching the explicit whole pattern (e.g. match "remotes/my/remote/with/slashes/" for remote "my/remote/with/slashes") but I doubt that is worth it for 99.9% of people.

The alternative regex that you are asking about is either using some syntax I am not familiar with or isn't quite correct. I'm most familiar with grep command line format, so perhaps tcl regex is different.

The original code does the equivalent of this:

~$ echo "remotes/origin/dev/test1" | grep -o "remotes/.*/" remotes/origin/dev/

The issue is that the '.*/' part is greedy in that it will match all the way up to and including the last /

My solution was to change the . to [^/] which means "any character but /". This stops the match at the first / after the remote name starts:

~$ echo "remotes/origin/dev/test1" | grep -o "remotes/[^/]*/" remotes/origin/

The alternative you suggested with '.*?/' doesn't seem to work with grep:

~$ echo "remotes/origin/dev/test1" | grep -o "remotes/.*?/" (no output, i.e. does not match)

Thank you.

Quoted 45 lines · David Holmer

Quoted 28 lines

Consider this example branch:

remotes/origin/master

gitk displays this branch with different background colors for each part:
"remotes/origin" in orange and "master" in green. The idea is to make it visually easy to read the branch name separately from the remote name.

However this fails when given this example branch:

remotes/origin/foo/bar

gitk displays this branch with "remotes/origin/foo" in orange and "bar" in green. This makes it hard to read the branch name "foo/bar". This is due to an inappropriately greedy regexp. This patch provides a fix so the same branch will now be displayed with "remotes/origin" in orange and "foo/bar" in green.

Signed-off-by David Holmer <[email protected]>

---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

gitk +0 -0

index 805a1c7..ca2392b 100755

gitk proc drawtags {id x xt y1} {

Quoted 9 lines

gitk +1 -2

@@ -6640,7 +6640,7 @@ proc drawtags {id x xt y1} {
            set xl [expr {$xl - $delta/2}]
            $canv create polygon $x $yt $xr $yt $xr $yb $x $yb \
                -width 1 -outline black -fill $col -tags tag.$id
-           if {[regexp {^(remotes/.*/|remotes/)} $tag match remoteprefix]} {
+           if {[regexp {^(remotes/[^/]*/|remotes/)} $tag match remoteprefix]} {
                set rwid [font measure mainfont $remoteprefix]
                set xi [expr {$x + 1}]
                set yti [expr {$yt + 1}]
--

This likely fixes the problem for most situations, but doesn't for a remote with a '/' in the name. Yet, I think this is a better state than the present.

Is the regex [^/]*/ more efficient than '.*?/`? Or do you find the former more readable?