diff mbox series

[v8,5/5] mergetools: add tools description to `git help config`

Message ID 20220329224439.290948-6-greenfoo@u92.eu (mailing list archive)
State Superseded
Headers show
Series vimdiff: new implementation with layout support | expand

Commit Message

Fernando Ramos March 29, 2022, 10:44 p.m. UTC
Now the output of `git help config` not only shows the name of each
tool (as before) but also a short description (as it is the case when
running `git mergetool --tool-help` or ` git difftool --tool-help`)

Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
 Documentation/Makefile | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Philippe Blain March 30, 2022, 12:43 p.m. UTC | #1
Hi Fernando,

Le 2022-03-29 à 18:44, Fernando Ramos a écrit :
> Now the output of `git help config` not only shows the name of each
> tool (as before) but also a short description (as it is the case when
> running `git mergetool --tool-help` or ` git difftool --tool-help`)
> 
> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>

This commit is authored by you but is missing your sign off. 
Also, I did not give my sign-off on this patch (that can't be assumed, 
it always has to be expressively given).

When people provide "in mail" diffs like I did in [1] and [2], it's usually meant
to be squashed into a commit of the series. In this case I think it would
be best squashed in 3/4, since as I explained in [3], the description of
each merge tool is already included in the 'git config' documentation after your
3/4, the below patch to the Makefile just makes it an Asciidoc "description list"
(and formats the merge tool names between backticks) instead of a plain list
as it was before.

Granted, after 3/4 only vimdiff and friends have a description, but that can be 
explained in the commit message, and the rest of the descriptions are added in 4/4.

> ---
>  Documentation/Makefile | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/Makefile b/Documentation/Makefile
> index 1eb9192dae..faed285462 100644
> --- a/Documentation/Makefile
> +++ b/Documentation/Makefile
> @@ -302,12 +302,12 @@ $(mergetools_txt): mergetools-list.made
>  
>  mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
>  	$(QUIET_GEN) \
> -	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
> +	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && TOOL_MODE=diff && \
>  		. ../git-mergetool--lib.sh && \
> -		show_tool_names can_diff "* " || :' >mergetools-diff.txt && \
> -	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
> +		show_tool_names can_diff' | sed -e "s/\([a-z0-9]*\)/\`\1\`;;/" >mergetools-diff.txt && \
> +	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && TOOL_MODE=merge && \
>  		. ../git-mergetool--lib.sh && \
> -		show_tool_names can_merge "* " || :' >mergetools-merge.txt && \
> +		show_tool_names can_merge' | sed -e "s/\([a-z0-9]*\)/\`\1\`;;/" >mergetools-merge.txt && \
>  	date >$@
>  
>  TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))
> 

Cheers,

Philippe.

[1] https://lore.kernel.org/git/1363db57-74de-226a-468f-69feffde6ba5@gmail.com/
[2] https://lore.kernel.org/git/d0a0d00b-5c1a-4a0c-a91c-b03403578f80@gmail.com/
[3] https://lore.kernel.org/git/f56a7a0b-8525-c4cc-7bc7-5ac4bba59206@gmail.com/
Fernando Ramos March 30, 2022, 6:33 p.m. UTC | #2
> This commit is authored by you but is missing your sign off.
> Also, I did not give my sign-off on this patch (that can't be assumed,
> it always has to be expressively given).

I see, sorry. As this patch is a verbatim copy of the one you provided in the
last message I thought it was not appropriate to put my name on it (as it does
not contain any line created by me)... but now I know that in these cases the
right thing to do is to squash into the commit being commented on and add a
"Helped-by:" note. Right?

I'll fix this in v9.

Just to double check, please confirm this is what you want me to do:

  1. Squash 5/5 into  3/5

  2. Update the commit message to:

     2.1 Explain that the description is also added to the output of `git help
         config`

     2.2 Remove your name from "Signed-off-by:"

     2.3 Keep you name in "Helped-by:"

Thanks!


PS: I must confess this whole process of sending patches to the git mailing
list brings me back memories from "Asterix: The 12 Tasks" [1] :) :) :)

[1] https://www.youtube.com/watch?v=ZHRGjfEQpy4
Philippe Blain March 30, 2022, 6:45 p.m. UTC | #3
Hi Fernando,

Le 2022-03-30 à 14:33, Fernando Ramos a écrit :
>> This commit is authored by you but is missing your sign off.
>> Also, I did not give my sign-off on this patch (that can't be assumed,
>> it always has to be expressively given).
> 
> I see, sorry. As this patch is a verbatim copy of the one you provided in the
> last message I thought it was not appropriate to put my name on it (as it does
> not contain any line created by me)... but now I know that in these cases the
> right thing to do is to squash into the commit being commented on and add a
> "Helped-by:" note. Right?

Yes, right. :)

> 
> I'll fix this in v9.
> 
> Just to double check, please confirm this is what you want me to do:
> 
>   1. Squash 5/5 into  3/5

yes. 

> 
>   2. Update the commit message to:
> 
>      2.1 Explain that the description is also added to the output of `git help
>          config`

Yes. For example, this is how I would phrase it, be free to copy 
(maybe as the second-to-last paragraph?):

Note that the function 'show_tool_names', used in the implmentation of
'git mergetool --tool-help', is also used in Documentation/Makefile to
generate the list of allowed values for the configuration variables 
'{diff,merge}.{gui,}tool'. Adjust the rule so its output is an Asciidoc
"description list" instead of a plain list, with the tool name as the item
and the newly added tool description as the description.

> 
>      2.2 Remove your name from "Signed-off-by:"
> 
>      2.3 Keep you name in "Helped-by:"

yes and yes.

> 
> Thanks!
> 
> 
> PS: I must confess this whole process of sending patches to the git mailing
> list brings me back memories from "Asterix: The 12 Tasks" [1] :) :) :)

Yes, it can be hard. I prefer using Gitgitgadget most of the time [1].

[1] https://gitgitgadget.github.io/
diff mbox series

Patch

diff --git a/Documentation/Makefile b/Documentation/Makefile
index 1eb9192dae..faed285462 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -302,12 +302,12 @@  $(mergetools_txt): mergetools-list.made
 
 mergetools-list.made: ../git-mergetool--lib.sh $(wildcard ../mergetools/*)
 	$(QUIET_GEN) \
-	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && TOOL_MODE=diff && \
 		. ../git-mergetool--lib.sh && \
-		show_tool_names can_diff "* " || :' >mergetools-diff.txt && \
-	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && \
+		show_tool_names can_diff' | sed -e "s/\([a-z0-9]*\)/\`\1\`;;/" >mergetools-diff.txt && \
+	$(SHELL_PATH) -c 'MERGE_TOOLS_DIR=../mergetools && TOOL_MODE=merge && \
 		. ../git-mergetool--lib.sh && \
-		show_tool_names can_merge "* " || :' >mergetools-merge.txt && \
+		show_tool_names can_merge' | sed -e "s/\([a-z0-9]*\)/\`\1\`;;/" >mergetools-merge.txt && \
 	date >$@
 
 TRACK_ASCIIDOCFLAGS = $(subst ','\'',$(ASCIIDOC_COMMON):$(ASCIIDOC_HTML):$(ASCIIDOC_DOCBOOK))