* [PATCH] gitk: Fix how remote branch names with / are drawn @ 2016-04-13 1:59 David Holmer 2016-04-13 11:35 ` Mike Rappazzo 0 siblings, 1 reply; 4+ messages in thread From: David Holmer @ 2016-04-13 1:59 UTC (permalink / raw) To: git; +Cc: 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 <odinguru@gmail.com> --- gitk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gitk b/gitk index 805a1c7..ca2392b 100755 --- a/gitk +++ b/gitk @@ -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}] -- 1.9.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] gitk: Fix how remote branch names with / are drawn 2016-04-13 1:59 [PATCH] gitk: Fix how remote branch names with / are drawn David Holmer @ 2016-04-13 11:35 ` Mike Rappazzo 2016-04-13 18:19 ` David Holmer 0 siblings, 1 reply; 4+ messages in thread From: Mike Rappazzo @ 2016-04-13 11:35 UTC (permalink / raw) To: David Holmer; +Cc: Git List On Tue, Apr 12, 2016 at 9:59 PM, David Holmer <odinguru@gmail.com> wrote: > 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 <odinguru@gmail.com> > --- > gitk | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gitk b/gitk > index 805a1c7..ca2392b 100755 > --- a/gitk > +++ b/gitk > @@ -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? ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gitk: Fix how remote branch names with / are drawn 2016-04-13 11:35 ` Mike Rappazzo @ 2016-04-13 18:19 ` David Holmer 2016-04-13 18:28 ` Mike Rappazzo 0 siblings, 1 reply; 4+ messages in thread From: David Holmer @ 2016-04-13 18:19 UTC (permalink / raw) To: Mike Rappazzo; +Cc: Git List 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. On Wed, Apr 13, 2016 at 7:35 AM, Mike Rappazzo <rappazzo@gmail.com> wrote: > On Tue, Apr 12, 2016 at 9:59 PM, David Holmer <odinguru@gmail.com> wrote: >> 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 <odinguru@gmail.com> >> --- >> gitk | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gitk b/gitk >> index 805a1c7..ca2392b 100755 >> --- a/gitk >> +++ b/gitk >> @@ -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? ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] gitk: Fix how remote branch names with / are drawn 2016-04-13 18:19 ` David Holmer @ 2016-04-13 18:28 ` Mike Rappazzo 0 siblings, 0 replies; 4+ messages in thread From: Mike Rappazzo @ 2016-04-13 18:28 UTC (permalink / raw) To: David Holmer; +Cc: Git List On Wed, Apr 13, 2016 at 2:19 PM, David Holmer <odinguru@gmail.com> wrote: > 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) `.*?` is a lazy match. I think it is an extended-regex, and your version is probably more efficient anyway. echo "remotes/origin/dev/test1" | grep -Eo "remotes/.*?/" > > > Thank you. > (Most people on this list don't like "top posting"), please try to reply inline instead. > On Wed, Apr 13, 2016 at 7:35 AM, Mike Rappazzo <rappazzo@gmail.com> wrote: >> On Tue, Apr 12, 2016 at 9:59 PM, David Holmer <odinguru@gmail.com> wrote: >>> 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 <odinguru@gmail.com> >>> --- >>> gitk | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/gitk b/gitk >>> index 805a1c7..ca2392b 100755 >>> --- a/gitk >>> +++ b/gitk >>> @@ -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? ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-04-13 18:29 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-04-13 1:59 [PATCH] gitk: Fix how remote branch names with / are drawn David Holmer 2016-04-13 11:35 ` Mike Rappazzo 2016-04-13 18:19 ` David Holmer 2016-04-13 18:28 ` Mike Rappazzo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).