diff mbox series

[v3,3/8] rebase-interactive: update 'merge' description

Message ID 669f4abd59e7dbb13281e85144d085180934fd15.1656422759.git.gitgitgadget@gmail.com (mailing list archive)
State Superseded
Headers show
Series rebase: update branches in multi-part topic | expand

Commit Message

Derrick Stolee June 28, 2022, 1:25 p.m. UTC
From: Derrick Stolee <derrickstolee@github.com>

The 'merge' command description for the todo list documentation in an
interactive rebase has multiple lines. The lines other than the first
one start with dots ('.') while the similar multi-line documentation for
'fixup' does not.

The 'merge' command was documented when interactive rebase was first
ported to C in 145e05ac44b (rebase -i: rewrite append_todo_help() in C,
2018-08-10). These dots might have been carried over from the previous
shell implementation.

The 'fixup' command was documented more recently in 9e3cebd97cb (rebase
-i: add fixup [-C | -c] command, 2021-01-29).

Looking at the output in an editor, my personal opinion is that the dots
are unnecessary and noisy. Remove them now before adding more commands
with multi-line documentation.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
 rebase-interactive.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Junio C Hamano June 28, 2022, 9 p.m. UTC | #1
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The 'merge' command description for the todo list documentation in an
> interactive rebase has multiple lines. The lines other than the first
> one start with dots ('.') while the similar multi-line documentation for
> 'fixup' does not.
>
> The 'merge' command was documented when interactive rebase was first
> ported to C in 145e05ac44b (rebase -i: rewrite append_todo_help() in C,
> 2018-08-10). These dots might have been carried over from the previous
> shell implementation.

The text indeed does come literally from the block removed by that
commit.  I wondered if the shell "gettext" in git-i18n.sh had some
magic in it, but it does not seem to have anything, and po/git.pot
around that time has these lines with leading dots, so I suspect
that they were quite deliberately added, not for the reasons of
formatting machinery (e.g. preventing somebody in the dataflow from
losing leading indentation), but to show them to the end users.

Unfortunately, the offending commit 4c68e7dd (sequencer: introduce
the `merge` command, 2018-04-25) does not justify them X-<.

> Looking at the output in an editor, my personal opinion is that the dots
> are unnecessary and noisy. Remove them now before adding more commands
> with multi-line documentation.

I personally do not mind having them in the UI, but I can also be
happy to see them go.  It is unlikely that any program is consuming
these strings, so I would say this is a fairly safe clean-up.

> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>  rebase-interactive.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index 87649d0c016..22394224faa 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -54,9 +54,9 @@ void append_todo_help(int command_count,
>  "l, label <label> = label current HEAD with a name\n"
>  "t, reset <label> = reset HEAD to a label\n"
>  "m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]\n"
> -".       create a merge commit using the original merge commit's\n"
> -".       message (or the oneline, if no original merge commit was\n"
> -".       specified); use -c <commit> to reword the commit message\n"
> +"        create a merge commit using the original merge commit's\n"
> +"        message (or the oneline, if no original merge commit was\n"
> +"        specified); use -c <commit> to reword the commit message\n"
>  "\n"
>  "These lines can be re-ordered; they are executed from top to bottom.\n");
>  	unsigned edit_todo = !(shortrevisions && shortonto);
Derrick Stolee June 29, 2022, 1:02 p.m. UTC | #2
On 6/28/22 5:00 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
> 
>> From: Derrick Stolee <derrickstolee@github.com>
>>
>> The 'merge' command description for the todo list documentation in an
>> interactive rebase has multiple lines. The lines other than the first
>> one start with dots ('.') while the similar multi-line documentation for
>> 'fixup' does not.
>>
>> The 'merge' command was documented when interactive rebase was first
>> ported to C in 145e05ac44b (rebase -i: rewrite append_todo_help() in C,
>> 2018-08-10). These dots might have been carried over from the previous
>> shell implementation.
> 
> The text indeed does come literally from the block removed by that
> commit.  I wondered if the shell "gettext" in git-i18n.sh had some
> magic in it, but it does not seem to have anything, and po/git.pot
> around that time has these lines with leading dots, so I suspect
> that they were quite deliberately added, not for the reasons of
> formatting machinery (e.g. preventing somebody in the dataflow from
> losing leading indentation), but to show them to the end users.
> 
> Unfortunately, the offending commit 4c68e7dd (sequencer: introduce
> the `merge` command, 2018-04-25) does not justify them X-<.
> 
>> Looking at the output in an editor, my personal opinion is that the dots
>> are unnecessary and noisy. Remove them now before adding more commands
>> with multi-line documentation.
> 
> I personally do not mind having them in the UI, but I can also be
> happy to see them go.  It is unlikely that any program is consuming
> these strings, so I would say this is a fairly safe clean-up.

Perhaps I should be a bit clearer that these are appearing in the
comment section of the todo-file when presented to the user for editing:

# Commands:
# p, pick <commit> = use commit
# r, reword <commit> = use commit, but edit the commit message
# e, edit <commit> = use commit, but stop for amending
# s, squash <commit> = use commit, but meld into previous commit
# f, fixup [-C | -c] <commit> = like "squash" but keep only the previous
#                    commit's log message, unless -C is used, in which case
#                    keep only this commit's message; -c is same as -C but
#                    opens the editor
# x, exec <command> = run command (the rest of the line) using shell
# b, break = stop here (continue rebase later with 'git rebase --continue')
# d, drop <commit> = remove commit
# l, label <label> = label current HEAD with a name
# t, reset <label> = reset HEAD to a label
# m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]
# .       create a merge commit using the original merge commit's
# .       message (or the oneline, if no original merge commit was
# .       specified); use -c <commit> to reword the commit message
#
# These lines can be re-ordered; they are executed from top to bottom.
#

This does not appear to be used anywhere else.

Thanks,
-Stolee
Phillip Wood June 30, 2022, 9:34 a.m. UTC | #3
On 28/06/2022 14:25, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <derrickstolee@github.com>
> 
> The 'merge' command description for the todo list documentation in an
> interactive rebase has multiple lines. The lines other than the first
> one start with dots ('.') while the similar multi-line documentation for
> 'fixup' does not.
> 
> The 'merge' command was documented when interactive rebase was first
> ported to C in 145e05ac44b (rebase -i: rewrite append_todo_help() in C,
> 2018-08-10). These dots might have been carried over from the previous
> shell implementation.
> 
> The 'fixup' command was documented more recently in 9e3cebd97cb (rebase
> -i: add fixup [-C | -c] command, 2021-01-29).
> 
> Looking at the output in an editor, my personal opinion is that the dots
> are unnecessary and noisy. Remove them now before adding more commands
> with multi-line documentation.

I agree, this is a nice cleanup

Thanks

Phillip

> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---
>   rebase-interactive.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/rebase-interactive.c b/rebase-interactive.c
> index 87649d0c016..22394224faa 100644
> --- a/rebase-interactive.c
> +++ b/rebase-interactive.c
> @@ -54,9 +54,9 @@ void append_todo_help(int command_count,
>   "l, label <label> = label current HEAD with a name\n"
>   "t, reset <label> = reset HEAD to a label\n"
>   "m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]\n"
> -".       create a merge commit using the original merge commit's\n"
> -".       message (or the oneline, if no original merge commit was\n"
> -".       specified); use -c <commit> to reword the commit message\n"
> +"        create a merge commit using the original merge commit's\n"
> +"        message (or the oneline, if no original merge commit was\n"
> +"        specified); use -c <commit> to reword the commit message\n"
>   "\n"
>   "These lines can be re-ordered; they are executed from top to bottom.\n");
>   	unsigned edit_todo = !(shortrevisions && shortonto);
Junio C Hamano June 30, 2022, 5:05 p.m. UTC | #4
Derrick Stolee <derrickstolee@github.com> writes:

> Perhaps I should be a bit clearer that these are appearing in the
> comment section of the todo-file when presented to the user for editing:

Yup, it is good to spell it out for others.

> This does not appear to be used anywhere else.

Nowhere in our codebase.  I do not think we have a way to know if
users (e.g. hooks and scripts) depend on them, but I am reasonably
sure that nobody would complain ;-)
diff mbox series

Patch

diff --git a/rebase-interactive.c b/rebase-interactive.c
index 87649d0c016..22394224faa 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -54,9 +54,9 @@  void append_todo_help(int command_count,
 "l, label <label> = label current HEAD with a name\n"
 "t, reset <label> = reset HEAD to a label\n"
 "m, merge [-C <commit> | -c <commit>] <label> [# <oneline>]\n"
-".       create a merge commit using the original merge commit's\n"
-".       message (or the oneline, if no original merge commit was\n"
-".       specified); use -c <commit> to reword the commit message\n"
+"        create a merge commit using the original merge commit's\n"
+"        message (or the oneline, if no original merge commit was\n"
+"        specified); use -c <commit> to reword the commit message\n"
 "\n"
 "These lines can be re-ordered; they are executed from top to bottom.\n");
 	unsigned edit_todo = !(shortrevisions && shortonto);