Re: [PATCH v3 1/2] git-am: emit usage when called w/o arguments and w/o patch on stdin

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 28, 2009, 18:52 UTC
Message-ID: <7v3af3s1oc.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1233154990-19745-1-git-send-email-jaysoffian@gmail.com>

Jay Soffian <jaysoffian@gmail.com> writes:

Show 10 quoted lines

> When git am is called w/o arguments, w/o a patch on stdin and the user hits
> ctrl-c, it leaves behind a partially populated $dotest directory. After this
> commit, it emits usage when called w/o arguments and w/o a patch on stdin.
>
> Noticed by Sverre Rabbelier
>
> Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
> ---
> Change from v2: make Junio happy by no longer removing $dotest if git-am is
> interupted while mailsplit is running.

Sorry, I think I was misleading. I did not mean that "Also" change was a bad thing to do. It just did not logically belong to this "is it coming from the tty?" issue.

While you are looking at "am", I have a complaint about a very rough and dangerous edge around a relatively new "usability" feature.

A scenario goes like this.

 (1) You have a patch, try to apply it to one existing topic branch

	$ git checkout myTopic
	$ git am some-patch

 (2) You see it does not apply, you realize that the patch is more suited
     for another branch.

	$ git checkout anotherTopic
        $ git am

 (3) It still does not apply, you examine the patch, and find flaws in its
     logic and decide not to apply it at all anywhere.

	$ git am --abort

After this, the tip of anotherTopic is updated to where the tip of myTopic was!

Being able to remove the state directory $GIT_DIR/rebase-apply from the command line is very useful when you want to get a clean slate to run another git-am session, and often it is handy to abort the application of the whole series when you do so (the latter is done by resetting the original branch back to the original commit). But being able to switch branches after starting application of patches from one mailbox is also useful in practice, so at least the latter should not be done blindly when the user asks for --abort. We definitely need some safety here.

Perhaps....

 (1) If --abort is asked for and you are on a branch different from where
     you started your "git am" session from, error out without doing
     anything.

 (2) Add --clean that does what the current --abort does, except for the
     rewinding of the HEAD.

The second one may be useful regardless of the "rough and dangerous edge" issue above. The early parts of the series may be useful and you may want to keep the result from their application, while discarding the remainder of the series.

Hmm?