Re: [PATCH] gitk: add "Hightlight commit name" menu entry
- From
- Raphaël Gallais-Pou <rgallaispou@gmail.com>
- Date
- Mar 23, 2024, 09:45 UTC
- Message-ID
- <146fbb87-9047-4d96-8d06-76f5710ca9e1@gmail.com>
- In-Reply-To
- <27b9c158-5cb1-46bf-851a-88a02448fa2d@xiplink.com>
Hi Marc,
Le 21/03/2024 à 16:51, Marc Branchaud a écrit :
Show 22 quoted lines
> > On 2024-01-30 03:53, Raphael Gallais-Pou wrote: >> When working with diverged branches, some patches can appear several >> times >> on different branches without having the need to merge those branches. >> On the other hand you may have to port a specific patch on another >> branch you are working on. The search with a SHA1 cannot be applied here >> since they would differ. >> >> This patch adds an entry in the main context menu to highlight every >> instance of a commit. > > Thanks for working on gitk! > > Unfortunately, I don't understand the description of your new option. > How is this different from the existing "Find containing:" feature? Gitk > can already highlights commits that match a specified string. Please > explain what gitk does when this new option is selected. > > Also, please explain how your code identifies "every instance" of a > commit. When I think of a "commit instance" I think of the > "git patch-id" command, which I don't see here.
It is based on the name of the commit. I agree that it is not ideal since the name can change between two versions.
As you stated below it is exactly a shortcut, and now that we are talking about it I think this is not the right right approach to do what I want.
I was not aware of the 'git patch-id' command, but this it clearly a better idea to base the search of the commit instances on it.
One thing that I wonder is that 'git patch-id' seems to be based on standard input. This means that in order to highlight every instances of a commit the algorithm would need to parse each and every patch and then proceed to hash them and compare to the one referenced.
Wouldn't it be a tad long to process ?
Show 20 quoted lines
>
> (It looks to me like this is basically a shortcut to auto-fill gitk's
> "containing:" field with the subject line of the selected commit?)
>
>> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
>> ---
>> gitk-git/gitk | 23 ++++++++++++++++++++---
>> 1 file changed, 20 insertions(+), 3 deletions(-)
>>
>> diff --git a/gitk-git/gitk b/gitk-git/gitk
>> index 7a087f123d..4b15230a16 100755
>> --- a/gitk-git/gitk
>> +++ b/gitk-git/gitk
>> @@ -2672,6 +2672,7 @@ proc makewindow {} {
>> {mc "Make patch" command mkpatch}
>> {mc "Create tag" command mktag}
>> {mc "Copy commit reference" command copyreference}
>> + {mc "Highlight commit name" command highlightcommitname}
>
> This line is indented with a tab, but it should use spaces.I will change my setup to use spaces instead of tabs one this file.
Show 38 quoted lines
>
>> {mc "Write commit to file" command writecommit}
>> {mc "Create new branch" command mkbranch}
>> {mc "Cherry-pick this commit" command cherrypick}
>> @@ -9002,13 +9003,13 @@ proc rowmenu {x y id} {
>> if {$id ne $nullid && $id ne $nullid2} {
>> set menu $rowctxmenu
>> if {$mainhead ne {}} {
>> - $menu entryconfigure 8 -label [mc "Reset %s branch to
>> here" $mainhead] -state normal
>> + $menu entryconfigure 9 -label [mc "Reset %s branch to
>> here" $mainhead] -state normal
>> } else {
>> - $menu entryconfigure 8 -label [mc "Detached head: can't
>> reset" $mainhead] -state disabled
>> + $menu entryconfigure 9 -label [mc "Detached head: can't
>> reset" $mainhead] -state disabled
>> }
>> - $menu entryconfigure 10 -state $mstate
>> $menu entryconfigure 11 -state $mstate
>> $menu entryconfigure 12 -state $mstate
>> + $menu entryconfigure 13 -state $mstate
>> } else {
>> set menu $fakerowmenu
>> }
>> @@ -9481,6 +9482,22 @@ proc copyreference {} {
>> clipboard append $reference
>> }
>> +proc highlightcommitname {} {
>> + global rowmenuid autosellen findstring gdttype
>> +
>> + set format "%s"
>> + set cmd [list git show -s --pretty=format:$format --date=short]
>
> Why bother with the $format variable here? Couldn't you just quote the
> --pretty part?
> "--pretty=format:%s"
> (FYI, I am not a TCL/TK coder.)I also am not a TCL developer. I pretty much duplicated the copyreference{} procedure to get what I wanted.
Best regards, Raphaël
Show 16 quoted lines
>
> M.
>
>> + if {$autosellen < 40} {
>> + lappend cmd --abbrev=$autosellen
>> + }
>> + set reference [eval exec $cmd $rowmenuid]
>> + set findstring $reference
>> + set gdttype [mc "containing:"]
>> +
>> + clipboard clear
>> + clipboard append $reference
>> +}
>> +
>> proc writecommit {} {
>> global rowmenuid wrcomtop commitinfo wrcomcmd NS