Re: [PATCH v3 2/2] docs: rewrite rebase --(no-)autosquash description

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 6, 2023, 11:07 UTC
Message-ID: <a4679ee0-b36f-441f-9350-b4c9c02f6e2d@gmail.com>
In-Reply-To: <20231105000808.10171-2-andy.koppe@gmail.com>

Hi Andy

On 05/11/2023 00:08, Andy Koppe wrote:

> Rewrite the description of the rebase --(no-)autosquash options to try
> to make it a bit clearer. Don't use "the '...'" to refer to part of a
> commit message,

Thanks for doing that, it is a definite improvement, I'm a bit concerned about the loss of detail in other areas though - see below.

Show 31 quoted lines

> mention how --interactive can be used to review the
> todo list, and add a bit more detail on commit --squash/amend.
> 
> Signed-off-by: Andy Koppe <andy.koppe@gmail.com>
> ---
>   Documentation/git-rebase.txt | 32 ++++++++++++++++++--------------
>   1 file changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 102ff91493..594158fcbc 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -589,21 +589,25 @@ See also INCOMPATIBLE OPTIONS below.
>   
>   --autosquash::
>   --no-autosquash::
> -	When the commit log message begins with "squash! ..." or "fixup! ..."
> -	or "amend! ...", and there is already a commit in the todo list that
> -	matches the same `...`, automatically modify the todo list of
> -	`rebase`, so that the commit marked for squashing comes right after
> -	the commit to be modified, and change the action of the moved commit
> -	from `pick` to `squash` or `fixup` or `fixup -C` respectively. A commit
> -	matches the `...` if the commit subject matches, or if the `...` refers
> -	to the commit's hash. As a fall-back, partial matches of the commit
> -	subject work, too. The recommended way to create fixup/amend/squash
> -	commits is by using the `--fixup`, `--fixup=amend:` or `--fixup=reword:`
> -	and `--squash` options respectively of linkgit:git-commit[1].
> +	Automatically squash commits with specially formatted messages into
> +	previous commits.  If a commit message starts with "squash! ",
> +	"fixup! " or "amend! ", the remainder of the subject line is taken
> +	as a commit specifier,

I like this change as I think saying "remainder of the subject line" is clearer than using `...`

> which matches a previous commit if it matches
> +	the start of the subject line or the hash of that commit.

This simplification glosses over some details, in particular it is no longer clear we prefer an exact subject match to a prefix match so that

	pick 123 foo
	pick 456 foo bar
	pick 789 fixup! foo

will be re-arranged to
	
	pick 123 foo
	fixup 789 fixup! foo
	pick 456 foo bar

and not

	pick 123 foo
	pick 456 foo bar
	fixup 789 fixup! foo

Show 7 quoted lines

>   +
> -If the `--autosquash` option is enabled by default using the
> -configuration variable `rebase.autoSquash`, this option can be
> -used to override and disable this setting.
> +In the rebase todo list, commits marked for squashing are moved right after
> +the commits they modify, and their action is changed from `pick` to `squash`,
> +`fixup` or `fixup -C`, depending on the squash marker.

Here we have lost the documentation that told users which marker corresponds to which action which is unfortunate.

Best Wishes

Phillip

Show 14 quoted lines

>  The `--interactive`
> +option can be used to review and edit the todo list before proceeding.
> ++
> +The recommended way to create commits with squash markers is by using the
> +`--squash`, `--fixup`, `--fixup=amend:` or `--fixup=reword:` options of
> +linkgit:git-commit[1], which take the target commit as an argument and
> +automatically fill in the subject line of the new commit from that.
> ++
> +The configuration variable `rebase.autoSquash` can be used to enable
> +`--autosquash` by default.  See the CONFIGURATION section below for details.
> +The `--no-autosquash` option overrides that setting.
>   +
>   See also INCOMPATIBLE OPTIONS below.
>