Re: [PATCH] gitk: Fix how remote branch names with / are drawn
- From
- David Holmer <odinguru@gmail.com>
- Date
- Apr 13, 2016, 18:19 UTC
- Message-ID
- <CAE8SKAMgZzyzoiy4JsqONN4wVWgVq-YmMn1+j2ZtELx+wJ1xEQ@mail.gmail.com>
- In-Reply-To
- <CANoM8SXixymz3=NQWgG5vSo7XDNh18_OzrNXU4+Y8CQ0LkB6sw@mail.gmail.com>
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:
Show 45 quoted lines
> 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?