git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Gabriel <g2p.code@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Make commit, cherry-pick and revert more silent.
Date: Sun, 06 Jan 2008 14:52:33 -0800	[thread overview]
Message-ID: <7vir2636tq.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1199634201-26013-1-git-send-email-g2p.code@gmail.com> (g2p.code@gmail.com's message of "Sun, 6 Jan 2008 16:43:21 +0100")

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.

> 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.

  reply	other threads:[~2008-01-06 22:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-06 15:43 [PATCH] Make commit, cherry-pick and revert more silent Gabriel
2008-01-06 22:52 ` Junio C Hamano [this message]
2008-01-07  8:55   ` Gabriel

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=7vir2636tq.fsf@gitster.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=g2p.code@gmail.com \
    --cc=git@vger.kernel.org \
    /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).