Re: [PATCH] gitk: Fix how remote branch names with / are drawn

[Mike Rappazzo <rappazzo@gmail.com>]

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?