Re: [PATCH v3 2/2] builtin/remote.c: show progress when renaming remote references

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2022, 23:20 UTC
Message-ID: <xmqqk0da7hdb.fsf@gitster.g>
In-Reply-To: <d5b0a4b71027619123b7284611692d3a9c128518.1646346287.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

> Instead of a more complex modification to the ref transaction code,
> display a progress meter when running verbosely in order to convince the
> user that Git is doing work while renaming a remote.

Is it still "when running verbosely"?

I thought that tying this to --[no-]progress was the whole point of iterating another round.

	... when the standard error output is connected to a
	terminal, so that user knows Git is not completely stuck.

> This is mostly done as-expected, with the minor caveat that we
> intentionally count symrefs renames twice, since renaming a symref takes
> place over two separate calls (one to delete the old one, and another to
> create the new one).

That's a nice note to leave here, as it is a bit tricky to reason about.

Show 5 quoted lines

> [1]: https://lore.kernel.org/git/572367B4.4050207@alum.mit.edu/
>
> Suggested-by: brian m. carlson <sandals@crustytoothpaste.net>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---

Show 18 quoted lines

>  Documentation/git-remote.txt |  2 +-
>  builtin/remote.c             | 30 +++++++++++++++++++++++++-----
>  t/t5505-remote.sh            |  4 +++-
>  3 files changed, 29 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt
> index 2bebc32566..cde9614e36 100644
> --- a/Documentation/git-remote.txt
> +++ b/Documentation/git-remote.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
>  [verse]
>  'git remote' [-v | --verbose]
>  'git remote add' [-t <branch>] [-m <master>] [-f] [--[no-]tags] [--mirror=(fetch|push)] <name> <URL>
> -'git remote rename' <old> <new>
> +'git remote rename' [--[no-]progress] <old> <new>
>  'git remote remove' <name>
>  'git remote set-head' <name> (-a | --auto | -d | --delete | <branch>)
>  'git remote set-branches' [--add] <name> <branch>...

Do we already have an entry for the --progress option in the description part of the documentation? I think the way progress works in the context of this command is pretty much bog-standard one that we may not have to, once the user has seen how progress options work elsewhere.

If not, then we'd want something like this squashed in, perhaps?

 Documentation/git-remote.txt | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git c/Documentation/git-remote.txt w/Documentation/git-remote.txt
index cde9614e36..e04810ee70 100644
--- c/Documentation/git-remote.txt
+++ w/Documentation/git-remote.txt
@@ -83,7 +83,10 @@ will always behave as if `--mirror` was passed.
 'rename'::
 
 Rename the remote named <old> to <new>. All remote-tracking branches and
-configuration settings for the remote are updated.
+configuration settings for the remote are updated.  When used interactively
+(i.e. the standard error stream is connected to a terminal),
+a progress bar may be shown while remote-tracking branches are renamed,
+which can be silenced by passing the `--no-progress` option.
 +
 In case <old> and <new> are the same, and <old> is a file under
 `$GIT_DIR/remotes` or `$GIT_DIR/branches`, the remote is converted to