git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Dragan Simic <dsimic@manjaro.org>
Cc: git@vger.kernel.org,  code@khaugsbakk.name,  rjusto@gmail.com,
	spectral@google.com
Subject: Re: [PATCH v2] branch: rework the descriptions of rename and copy operations
Date: Fri, 16 Feb 2024 11:59:02 -0800	[thread overview]
Message-ID: <xmqq1q9ci3jt.fsf@gitster.g> (raw)
In-Reply-To: <6e1c3f2c5816f09aab561bc7dec2b7455d70aaec.1708087213.git.dsimic@manjaro.org> (Dragan Simic's message of "Fri, 16 Feb 2024 13:44:19 +0100")

Dragan Simic <dsimic@manjaro.org> writes:

> Move the descriptions of the <oldbranch> and <newbranch> arguments to the
> descriptions of the branch rename and copy operations, where they naturally
> belong.  Also, improve the descriptions of these two branch operations and,
> for completeness, describe the outcomes of forced operations.
>
> Describing the arguments together with their respective operations, instead
> of describing them separately in a rather unfortunate attempt to squeeze more
> meaning out of fewer words, flows much better and makes the git-branch(1)
> man page significantly more usable.

The intention to remove non-option from the OPTIONS enumeration,
and to explain <new> and <old> used as arguments to -m and -c where
these options are described, are both very good (heh, after all,
they are parts of what I envisioned to be the way to go in the
longer term ;-).

>  overridden by using the `--track` and `--no-track` options, and
>  changed later using `git branch --set-upstream-to`.
>  
> -With a `-m` or `-M` option, <oldbranch> will be renamed to <newbranch>.
> -If <oldbranch> had a corresponding reflog, it is renamed to match
> -<newbranch>, and a reflog entry is created to remember the branch
> -renaming. If <newbranch> exists, -M must be used to force the rename
> -to happen.
> -
> -The `-c` and `-C` options have the exact same semantics as `-m` and
> -`-M`, except instead of the branch being renamed, it will be copied to a
> -new name, along with its config and reflog.
> -
>  With a `-d` or `-D` option, `<branchname>` will be deleted.  You may
>  specify more than one branch for deletion.  If the branch currently
>  has a reflog then the reflog will also be deleted.

But the halfway modification to the description section in this
patch is not an improvement.  It makes some options described there
while -m and -c are completely missing now, making the section
incomplete and coverage of the operating modes of the command
uneven.  

> +-m [<oldbranch>] <newbranch>::
> +--move [<oldbranch>] <newbranch>::
> +	Rename an existing branch `<oldbranch>` to `<newbranch>`;  if left
> +	unspecified, `<oldbranch>` defaults to the current branch.  The
> +	configuration variables for the `<oldbranch>` branch and its reflog
> +	are also renamed appropriately to be used with `<newbranch>`.  In
> +	addition, a reflog entry is created to remember the branch renaming.
> +	Renaming fails if branch `<newbranch>` already exists, but `-M`
> +	or `--move --force` can be used to overwrite the contents of the
> +	existing branch `<newbranch>` while renaming.

OK.  This is way more readable than the previous attempts we made.

The description of the single failure mode still worries me (see my
previous message on this).  Here is my attempt:

	When the command fails due to an existing '<newbranch>', you
	can use `-M` (or `--move --force`) to force overwriting it.

to hint that there may be other ways for the command to fail, and
hint that `-M` may not always resolve issues, but I do not know how
successful it is.  I could add

	Note that `-M <old> <new>` will not resolve an error if the
	reason why `-m` fails is to protect the other worktree that
	checks out (or otherwise uses) <old> and <new> points at a
	different commit.

but we do not necessarily want to appear to be exhaustive here, so,
I dunno.

> +-M [<oldbranch>] <newbranch>::
>  	Shortcut for `--move --force`.

OK.

> +--copy [<oldbranch>] <newbranch>::
> +	Copy an existing branch `<oldbranch>` to `<newbranch>`;  if left
> +	unspecified, `<oldbranch>` defaults to the current branch.  The
> +	configuration variables for the `<oldbranch>` branch and its reflog
> +	are also copied appropriately to be used with `<newbranch>`.
> +	Copying fails if branch `<newbranch>` already exists, but `-C`
> +	or `--copy --force` can be used to overwrite the contents of the
> +	existing branch `<newbranch>` while copying.

Exactly the same comment on "other failure modes" applies here.

> -<oldbranch>::
> -	The name of an existing branch.  If this option is omitted,
> -	the name of the current branch will be used instead.
> -
> -<newbranch>::
> -	The new name for an existing branch. The same restrictions as for
> -	<branchname> apply.
> -

Removals of these lines are very pleasing ;-).

  reply	other threads:[~2024-02-16 19:59 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-16 12:44 [PATCH v2] branch: rework the descriptions of rename and copy operations Dragan Simic
2024-02-16 19:59 ` Junio C Hamano [this message]
2024-02-16 21:20   ` Dragan Simic
2024-02-16 21:45     ` Junio C Hamano
2024-02-16 21:58       ` Dragan Simic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq1q9ci3jt.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=code@khaugsbakk.name \
    --cc=dsimic@manjaro.org \
    --cc=git@vger.kernel.org \
    --cc=rjusto@gmail.com \
    --cc=spectral@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).