[PATCH] Make commit, cherry-pick and revert more silent.

Gabriel· Jan 6, 2008, 15:43 UTC · lore

Commit now obeys --quiet more.
Cherry-pick and revert call commit as --quiet.
Prevents us from displaying working-tree status once or even twice.
---
 builtin-commit.c |    4 +++-
 builtin-revert.c |    6 ++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

Show changes to 2 files +7 −3

builtin-commit.c, builtin-revert.c

diff --git a/builtin-commit.c b/builtin-commit.c
index 73f1e35..96ace77 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -759,7 +759,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	if (!prepare_log_message(index_file, prefix) && !in_merge &&
 	    !allow_empty && !(amend && is_a_merge(head_sha1))) {
-		run_status(stdout, index_file, prefix, 0);
+		fprintf(stderr, "There are no changes, not committing.\n");
+		if (!quiet)
+			run_status(stdout, index_file, prefix, 0);
 		rollback_index_files();
 		unlink(commit_editmsg);
 		return 1;
diff --git a/builtin-revert.c b/builtin-revert.c
index 4bf8eb2..b925358 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -392,9 +392,11 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 
 	if (!no_commit) {
 		if (edit)
-			return execl_git_cmd("commit", "-n", NULL);
+			return execl_git_cmd("commit", "--quiet",
+				"-n", NULL);
 		else
-			return execl_git_cmd("commit", "-n", "-F", defmsg, NULL);
+			return execl_git_cmd("commit", "--quiet",
+				"-n", "-F", defmsg, NULL);
 	}
 	if (reencoded_message)
 		free(reencoded_message);

-- 
1.5.4.rc2.39.g00d2

Junio C Hamano· Jan 6, 2008, 22:52 UTC · re: Gabriel · lore

Re: [PATCH] Make commit, cherry-pick and revert more silent.

Gabriel <g2p.code@gmail.com> writes:

> Commit now obeys --quiet more.
> Cherry-pick and revert call commit as --quiet.
> Prevents us from displaying working-tree status once or even twice.

Well, you also need to defend that it is a good thing not to show the status information during cherry-pick or revert much better, especially when we are this late into the -rc cycle.

Show 12 quoted lines

> diff --git a/builtin-commit.c b/builtin-commit.c
> index 73f1e35..96ace77 100644
> --- a/builtin-commit.c
> +++ b/builtin-commit.c
> @@ -759,7 +759,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
>  
>  	if (!prepare_log_message(index_file, prefix) && !in_merge &&
>  	    !allow_empty && !(amend && is_a_merge(head_sha1))) {
> -		run_status(stdout, index_file, prefix, 0);
> +		fprintf(stderr, "There are no changes, not committing.\n");
> +		if (!quiet)
> +			run_status(stdout, index_file, prefix, 0);

Especially if you are introducing a change to a command you do not even mention in the topic line of the patch.

Having said that, I think it is a good change, if the UI change to cherry-pick and revert only triggers when the operation did not have any effect.

It is much harder to know for a user if a cherry-pick or revert will result in such a situation before actually running these commands, than when making his own commit. And the status output from underlying git-commit only distracts him by obscuring the punch-line "nothing added to commit", which is currently the only clue. I'd agree that is a UI bug in the current implementation of cherry-pick/revert.

So a more convincing presentation would be:

 * A single patch to "git commit".  The commit log message would
   read:

   When there is nothing to commit, "git commit --quiet" still
   shows the status output before saying "nothing added to
   commit...".  This patch makes it less verbose.

 * Another patch on top of it that runs "git commit" with
   the "--quiet" option from cherry-pick and revert.  The commit
   log message would read:

   When cherry-pick or revert results in no change at all
   (e.g. the user cherry-picked an ancestor of the current
   commit), the command correctly refuses to create a new
   commit, but it responds by showing the status output and
   "nothing added to commit" message.

   This is a very roundabout way to tell the user that the
   cherry-pick or revert was unnecessary.  Especially because it
   is much harder to know for a user if a cherry-pick or revert
   will result in such a situation before actually running these
   commands, than when making his own commit.

   This patch makes cherry-pick and revert call "git commit"
   with --quiet option to make the output much less confusing.

After I wrote all that, I realized that the patch is not acceptable as is.

Why?

This makes a successful cherry-pick way too silent. With your patch, we will see:

 * "Auto-merged ..." messages that shows what paths are affected
   by the cherry-pick/revert (which I do not think we would want
   to squelch),

 * "Finished one cherry-pick."

But we will lose the "Created commit ...: <msg>" and "<num> files changed..." summary, neither of which we would want to lose.

Also sign your patch (see Documentation/SubmittingPatches), please, when you try the second round.

Thanks.

Gabriel· Jan 7, 2008, 08:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make commit, cherry-pick and revert more silent.

Le Sun, 06 Jan 2008 14:52:33 -0800, Junio C Hamano a écrit :

> So a more convincing presentation would be:
> ...

Will split.

Show 16 quoted lines

> After I wrote all that, I realized that the patch is not acceptable as
> is.
> 
> Why?
> 
> This makes a successful cherry-pick way too silent.  With your patch, we
> will see:
> 
>  * "Auto-merged ..." messages that shows what paths are affected
>    by the cherry-pick/revert (which I do not think we would want to
>    squelch),
> 
>  * "Finished one cherry-pick."
> 
> But we will lose the "Created commit ...: <msg>" and "<num> files
> changed..."  summary, neither of which we would want to lose.

I'll have a look. This seems to require refactoring the interface between cherry-pick/ revert and commit.

> Also sign your patch (see Documentation/SubmittingPatches), please, when
> you try the second round.
> 
> Thanks.

Thanks, I appreciate the review.

patchMake commit, cherry-pick and revert more silent.