Re: [PATCH] gitk: add cherry-pick -x context menu action

[Johannes Sixt <j6t@kdbg.org>]

Please paste the contents of the format-patch output into the email; do
not send it as attachment. See the format-patch man page[1] how to set
up a mail client in a way that does not mangle the patch text.

[1] https://git-scm.com/docs/git-format-patch#_mua_specific_hints

Am 24.10.26 um 11:40 schrieb Ralf Habacker:
> For example, this command is very useful for cherry-picking
> commits into a stable branch using a reference—something that

We need an en-dash or em-dash (two or three dashes) between "reference"
and "something" to give the sentence the correct meaning.

> otherwise can only be done from the command line.
> 

Personally, I use cherry-pick -x so rarely that I have no problem going
to the command line for those cases. Since the context menu is already
very long, I am hesitant to add a small variation of a command that is
already present (and very useful).

Are others interested in this feature?

Below some comments on the implementation just in case there are.

> Signed-off-by: Ralf Habacker <ralf.habacker@freenet.de>
> ---
>  gitk-git/gitk        | 15 ++++++++++-----
>  gitk-git/po/bg.po    |  3 +++
>  gitk-git/po/ca.po    |  3 +++
>  gitk-git/po/de.po    |  3 +++
>  gitk-git/po/es.po    |  3 +++
>  gitk-git/po/fr.po    |  3 +++
>  gitk-git/po/hu.po    |  3 +++
>  gitk-git/po/it.po    |  3 +++
>  gitk-git/po/ja.po    |  3 +++
>  gitk-git/po/pt_br.po |  3 +++
>  gitk-git/po/pt_pt.po |  3 +++
>  gitk-git/po/ru.po    |  3 +++
>  gitk-git/po/sv.po    |  3 +++
>  gitk-git/po/ta.po    |  3 +++
>  gitk-git/po/vi.po    |  3 +++
>  gitk-git/po/zh_cn.po |  3 +++

Please do not edit the translation files.

>  16 files changed, 55 insertions(+), 5 deletions(-)
> 
> diff --git a/gitk-git/gitk b/gitk-git/gitk
> index 9506739c50..5d9c781778 100755
> --- a/gitk-git/gitk
> +++ b/gitk-git/gitk
> @@ -2859,6 +2859,7 @@ proc makewindow {} {
>          {mc "Write commit to file" command writecommit}
>          {mc "Create new branch" command mkbranch}
>          {mc "Cherry-pick this commit" command cherrypick}
> +        {mc "Cherry-pick this commit with -x" command {cherrypick 1}}

We are in a UI here. Avoid jargon. ("What is -x??") How about:

	"Cherry-pick with cross-reference"

>          {mc "Reset HEAD branch to here" command resethead}
>          {mc "Mark this commit" command markhere}
>          {mc "Return to mark" command gotomark}
> @@ -9167,13 +9168,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
>      }
> @@ -9881,7 +9882,7 @@ proc exec_citool {tool_args {baseid {}}} {
>      array set env $save_env
>  }
>  
> -proc cherrypick {} {
> +proc cherrypick {{withx 0}} {

If you make the default value of the parameter "-r", then the new caller
can just pass "-x" and we do not need the conditional below. (Also
rename the parameter.)

>      global rowmenuid curview
>      global mainhead mainheadid
>      global gitdir
> @@ -9898,7 +9899,11 @@ proc cherrypick {} {
>      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 {safe_exec [list sh -c "git cherry-pick -r $rowmenuid 2>&1"]} err]} {
> +        set xopt ""
> +    if {$withx} {
> +        set xopt " -x"
> +    }
> +    if {[catch {safe_exec [list sh -c "git cherry-pick -r$xopt $rowmenuid 2>&1"]} err]} {

-r and -x are opposites. While the last one wins, it would be great if
the one option that is not used would not remain in the command.

>          notbusy cherrypick
>          if {[regexp -line \
>                   {Entry '(.*)' (would be overwritten by merge|not uptodate)} \

-- Hannes