Re: [PATCH v2 07/10] gitk: add keyboard bind for cherry-pick command
Show 5 quoted lines
>> + {mc "Create new branch" command {mkbranch $rowmenuid}}
>> + {mc "Cherry-pick this commit" command {cherrypick $rowmenuid}}
>
> The change regarding Create new branch is not related to this commit's
> topic and should be elsewhere.Sorry, sloppy mistake, will fix, thanks.
Show 6 quoted lines
>> + if {! [info exists headids($mainhead)]} {
>> + error_popup [mc "Cannot cherry-pick to a detached head"]
>> + return
>> + }
>
> Why is it necessary to forbid this now? It was not forbidden before.Oh, I had no idea you can actually cherry-pick to a detached head! When a added this check I though I fixed an existing bug, which only manifested if the user had checked out a detached head using the terminal and then used cherry-pick in gitk. I now see that it is not an error.
I have mixed feelings about allowing this, but it's probably best to not invent artificial limitations in gitk, so I'll remove the check.
/Jens
On 2023-07-05 22:07, Johannes Sixt wrote:
Show 91 quoted lines
> Am 03.07.23 um 20:45 schrieb Jens Lidestrom via GitGitGadget:
>> From: Jens Lidestrom <jens@lidestrom.se>
>>
>> Signed-off-by: Jens Lidestrom <jens@lidestrom.se>
>> ---
>> gitk-git/gitk | 23 +++++++++++++++--------
>> 1 file changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/gitk-git/gitk b/gitk-git/gitk
>> index 65ca11becca..351b88f10c0 100755
>> --- a/gitk-git/gitk
>> +++ b/gitk-git/gitk
>> @@ -2690,6 +2690,7 @@ proc makewindow {} {
>> bind $ctext $ctxbut {pop_diff_menu %W %X %Y %x %y}
>> bind $ctext <Button-1> {focus %W}
>> bind $ctext <<Selection>> rehighlight_search_results
>> + bind . <$M1B-p> {cherrypick [selected_line_id]}
>> bind . <$M1B-t> {resethead [selected_line_id]}
>> bind . <$M1B-o> {checkout [selected_line_heads] [selected_line_id]}
>> bind . <$M1B-m> {rmbranch [selected_line_heads] [selected_line_id] 1}
>> @@ -2710,8 +2711,8 @@ proc makewindow {} {
>> {mc "Create tag" command mktag}
>> {mc "Copy commit reference" command copyreference}
>> {mc "Write commit to file" command writecommit}
>> - {mc "Create new branch" command mkbranch}
>> - {mc "Cherry-pick this commit" command cherrypick}
>> + {mc "Create new branch" command {mkbranch $rowmenuid}}
>> + {mc "Cherry-pick this commit" command {cherrypick $rowmenuid}}
>
> The change regarding Create new branch is not related to this commit's
> topic and should be elsewhere.
>
>> {mc "Reset current branch to here" command {resethead $rowmenuid}}
>> {mc "Mark this commit" command markhere}
>> {mc "Return to mark" command gotomark}
>> @@ -3186,6 +3187,7 @@ proc keys {} {
>> [mc "<%s-minus> Decrease font size" $M1T]
>> [mc "<F5> Update"]
>> [mc "<%s-T> Reset current branch to selected commit" $M1T]
>> +[mc "<%s-P> Cherry-pick selected commit to current branch" $M1T]
>> [mc "<%s-O> Check out selected commit" $M1T]
>> [mc "<%s-B> Create branch on selected commit" $M1T]
>> [mc "<%s-M> Remove selected branch" $M1T]
>> @@ -9758,24 +9760,29 @@ proc exec_citool {tool_args {baseid {}}} {
>> array set env $save_env
>> }
>>
>> -proc cherrypick {} {
>> - global rowmenuid curview
>> +proc cherrypick {id} {
>> + global curview headids
>> global mainhead mainheadid
>> global gitdir
>>
>> + if {! [info exists headids($mainhead)]} {
>> + error_popup [mc "Cannot cherry-pick to a detached head"]
>> + return
>> + }
>
> Why is it necessary to forbid this now? It was not forbidden before.
>
>> +
>> set oldhead [exec git rev-parse HEAD]
>> - set dheads [descheads $rowmenuid]
>> + set dheads [descheads $id]
>> if {$dheads ne {} && [lsearch -exact $dheads $oldhead] >= 0} {
>> set ok [confirm_popup [mc "Commit %s is already\
>> included in branch %s -- really re-apply it?" \
>> - [string range $rowmenuid 0 7] $mainhead]]
>> + [string range $id 0 7] $mainhead]]
>> if {!$ok} return
>> }
>> nowbusy cherrypick [mc "Cherry-picking"]
>> update
>> # Unfortunately git-cherry-pick writes stuff to stderr even when
>> # no error occurs, and exec takes that as an indication of error...
>> - if {[catch {exec sh -c "git cherry-pick -r $rowmenuid 2>&1"} err]} {
>> + if {[catch {exec sh -c "git cherry-pick -r $id 2>&1"} err]} {
>> notbusy cherrypick
>> if {[regexp -line \
>> {Entry '(.*)' (would be overwritten by merge|not uptodate)} \
>> @@ -9791,7 +9798,7 @@ proc cherrypick {} {
>> resolve it?"]]} {
>> # Force citool to read MERGE_MSG
>> file delete [file join $gitdir "GITGUI_MSG"]
>> - exec_citool {} $rowmenuid
>> + exec_citool {} $id
>> }
>> } else {
>> error_popup $err
>