Re: [PATCH v2 1/1] t/lib-rebase: improve documentation of set_fake_editor()

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 9, 2023, 19:38 UTC
Message-ID: <xmqqfs4sm2fu.fsf@gitster.g>
In-Reply-To: <20230809171531.2564785-2-oswald.buddenhagen@gmx.de>

Oswald Buddenhagen <oswald.buddenhagen@gmx.de> writes:

Show 18 quoted lines

> Firstly, make it reflect better what actually happens. Not omitting some
> possibilities makes it easier to fully exploit them, and not
> contradicting the implementation makes it easier to grok and thus modify
> the code.
>
> Secondly, improve the overall structure, putting more general info
> further up.
>
> Thirdly, document `merge`, `fakesha`, and `break`, which were previously
> omitted entirely.
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
>
> ---
> v2:
> - everything phillip asked for, i think. of course, it's still bullet
>   points, just without the punctuation - i wouldn't know what else to
>   do about it which would be an actual improvement.

If more bits are spent for the explanation of concrete changes (like "move general info first and then describe details") that back subjective claims (e.g. make it "better", "improve" the structure), the result probably would read better. Readers do not have to take your word on it that the result is "better", but at least would understand the reason why you think the result is "better" and can agree (or disagree) more easily.

I am not Phillip but to me what you wrote in the above look clear enough. Thanks for working on it.

Show 40 quoted lines

> Cc: Phillip Wood <phillip.wood123@gmail.com>
> ---
>  t/lib-rebase.sh | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> index 7ca5b918f0..133c856992 100644
> --- a/t/lib-rebase.sh
> +++ b/t/lib-rebase.sh
> @@ -8,18 +8,21 @@
>  # - check that non-commit messages have a certain line count with $EXPECT_COUNT
>  # - check the commit count in the commit message header with $EXPECT_HEADER_COUNT
>  # - rewrite a rebase -i script as directed by $FAKE_LINES.
> -#   $FAKE_LINES consists of a sequence of words separated by spaces.
> -#   The following word combinations are possible:
> +#   $FAKE_LINES consists of a sequence of words separated by spaces;
> +#   spaces inside the words are encoded as underscores.
> +#   The following words are possible:
>  #
> -#   "<lineno>" -- add a "pick" line with the SHA1 taken from the
> -#       specified line.
> +#   "<cmd>" -- override the command for the next line specification. Can be
> +#       "pick", "squash", "fixup[_-(c|C)]", "edit", "reword", "drop",
> +#       "merge[_-{c|C}_<rev>]", or "bad" for an invalid command.
>  #
> -#   "<cmd> <lineno>" -- add a line with the specified command
> -#       ("pick", "squash", "fixup"|"fixup_-C"|"fixup_-c", "edit", "reword" or "drop")
> -#       and the SHA1 taken from the specified line.
> +#   "<lineno>" -- add a command, using the specified line as a template.
> +#       If the command has not been overridden, the line will be copied
> +#       verbatim, usually resulting in a "pick" line.
>  #
> -#   "_" -- add a space, like "fixup_-C" implies "fixup -C" and
> -#       "exec_cmd_with_args" add an "exec cmd with args" line.
> +#   "fakesha" -- add a command ("pick" by default), using a fake SHA1.
> +#
> +#   "exec_[command...]", "break" -- add the specified command.
>  #
>  #   "#" -- Add a comment line.
>  #