git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Elijah Newren" <newren@gmail.com>
To: "Theodore Tso" <tytso@mit.edu>
Cc: "Sam Vilain" <sam@vilain.net>,
	git@vger.kernel.org, "Sam Vilain" <samv@vilain.net>
Subject: Re: [PATCH] Documentation: add a planning document for the next CLI revamp
Date: Sat, 1 Nov 2008 13:26:27 -0600	[thread overview]
Message-ID: <51419b2c0811011226p3369b7f7t37b98032ca8dd9ac@mail.gmail.com> (raw)
In-Reply-To: <20081030143918.GB14744@mit.edu>

Hi,

On Thu, Oct 30, 2008 at 8:39 AM, Theodore Tso <tytso@mit.edu> wrote:
> Here are my favorites:
>
> * Add the command "git revert-file <files>" which is syntactic sugar for:
>
>        git checkout HEAD -- <files>
>
>  Rationale: Many other SCM's have a way of undoing local edits to a
>  file very simply, i.e."hg revert <file>" or "svn revert <file>", and
>  for many developers's workflow, it's useful to be able to undo local
>  edits to a single file, but not to everything else in the working
>  directory.  And "git checkout HEAD -- <file>" is rather cumbersome
>  to type, and many beginning users don't find it intuitive to look in
>  the "git-checkout" man page for instructions on how to revert a
>  local file.

I agree with the rationale, but the suggested implementation (as with
the original suggestion for "git undo") is somewhat problematic.  I
have a write-up somewhere documenting the ways various individual git
commands fail to be an appropriate replacement for svn/hg/bzr
revert[1], but in short the "git checkout HEAD -- <file>"
implementation for svn/hg/bzr-like revert fails in the following ways:
  * It does not work for the initial commit
  * It won't untrack or remove files (this is related to the previous
and following items)
  * It doesn't allow reverting a file or directory to a revision prior
to HEAD (making it like svn; note though that both bzr and hg have
such an option and I have found it handy a few times)
  * It's inappropriate to use during an incomplete merge.

The incomplete merge case is particularly interesting.  If the user
specifies a file or subdirectory, they should also specify a branch to
revert relative to (and it should be an error if they don't).  If the
user specifies "." then there's the question of whether they are
attempting to undo the merge (meaning that .git/MERGE_MSG and
.git/MERGE_HEAD should be removed).

Just as food for thought, here's what eg does in the incomplete merge case:

$ eg revert foo
Aborting: Cannot revert the changes since the last commit, since you are in
the middle of a merge and there are multiple last commits.  Please add
  --since BRANCH
to your flags to eg revert, where BRANCH is one of
  master, devel
If you simply want to abort your merge and undo its conflicts, run
  eg revert --since HEAD


There's a couple more issues here that I could go on about, but I'll
mention just one more thing for this email:  Since users often get
confused between different kinds of "reverting" or "undoing", a plain
'eg revert' is also pretty helpful in a wide variety of circumstances
(it always aborts with an error message, but one that detects what the
user might want and suggests appropriate commands in the various
cases.)

Elijah


[1] There are a number of different commands that people suggest for
new users to replace other systems' revert behavior, but each has
areas in which it will fail to do what users expect or do additional
things users don't want (including discarding data)  Interestingly,
I've tried four different alternative git porcelains and each one
implemented their svn/hg/bzr-like revert incorrectly.  One of these
was EasyGit, in which I got it wrong not once but three separate
times.  (And if alternative porcelain authors can't easily get it
right, we clearly can't expect normal users to know how to do so; I
think this is a pretty good argument for providing a function for this
behavior in core git.)  I think I finally have it implemented
correctly now in EasyGit, after my fourth try...

  parent reply	other threads:[~2008-11-01 19:27 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-30  3:48 [PATCH] Documentation: add a planning document for the next CLI revamp Sam Vilain
2008-10-30 10:55 ` Stefan Karpinski
2008-10-31 11:38   ` Kyle Moffett
2008-10-30 13:24 ` Pierre Habouzit
2008-10-30 15:25   ` Julian Phillips
2008-10-31  0:34   ` Jeff King
2008-11-02 21:53   ` Junio C Hamano
2008-11-03 13:47     ` Pierre Habouzit
2008-10-30 14:34 ` Nicolas Pitre
2008-10-30 14:52   ` Shawn O. Pearce
2008-10-30 14:59     ` Mike Hommey
2008-10-30 15:01       ` Pierre Habouzit
2008-10-30 16:53         ` Nicolas Pitre
2008-10-30 17:31           ` Sam Vilain
2008-10-30 18:28             ` Nicolas Pitre
2008-10-30 22:46               ` Yann Dirson
2008-10-30 23:28               ` Sam Vilain
2008-10-30 23:55             ` Jakub Narebski
2008-10-31  6:51               ` Sam Vilain
2008-10-31  7:36                 ` Jakub Narebski
2008-11-03  8:43                   ` Sam Vilain
2008-11-03 12:06                     ` Jakub Narebski
2008-11-01  0:37         ` Johannes Schindelin
2008-10-30 14:39 ` Theodore Tso
2008-10-30 14:43   ` Pierre Habouzit
2008-10-30 16:30     ` Theodore Tso
2008-10-30 16:43       ` Pierre Habouzit
2008-10-30 17:44         ` Sam Vilain
2008-10-30 17:03       ` Nicolas Pitre
2008-11-02  6:08       ` Junio C Hamano
2008-11-02 10:09         ` Theodore Tso
2008-10-30 15:02   ` Andreas Ericsson
2008-11-01 19:57     ` Elijah Newren
2008-10-30 15:20   ` Matthieu Moy
2008-10-30 17:00     ` Nicolas Pitre
2008-10-30 17:03       ` Pierre Habouzit
2008-10-30 17:17         ` Nicolas Pitre
2008-10-30 18:06           ` Sam Vilain
2008-11-02 22:17         ` Junio C Hamano
2008-11-03  6:01           ` Sam Vilain
2008-11-01 19:42     ` Elijah Newren
2008-10-30 17:51   ` Sam Vilain
2008-10-30 23:27     ` Theodore Tso
2008-11-01 20:27     ` Elijah Newren
2008-11-02  1:06       ` Theodore Tso
2008-11-02  4:41         ` Elijah Newren
2008-11-01 19:26   ` Elijah Newren [this message]
2008-11-02 22:01   ` Junio C Hamano
2008-11-01 18:36 ` Elijah Newren
     [not found] <20081030002239.D453B21D14E@mail.utsl.gen.nz>
2008-10-31  0:31 ` Jeff King
2008-10-31  6:40   ` Sam Vilain
2008-10-31  8:20     ` Pierre Habouzit
2008-11-02  4:18     ` Jeff King
2008-11-02  9:56       ` Theodore Tso
2008-10-31 16:46   ` Johannes Schindelin
2008-11-02  3:42     ` Jeff King
2008-11-02  3:53   ` Jeff King
2008-11-02 22:27   ` Junio C Hamano
2008-11-03  5:59     ` Sam Vilain
2008-11-03  9:48       ` Jakub Narebski
2008-11-03  9:53         ` Sverre Rabbelier
2008-11-04  9:18       ` Dmitry Potapov
2008-11-04 18:10         ` Sam Vilain
2008-11-04 19:46           ` Junio C Hamano
2008-11-05  3:05             ` Jeff King
2008-11-05  6:40               ` Junio C Hamano
2008-11-05 22:53           ` Dmitry Potapov
2008-11-03  6:56     ` Jeff King
2008-11-03  6:59       ` Jeff King
2008-11-03  9:25     ` Pierre Habouzit
2008-11-03 23:33       ` Junio C Hamano
2008-11-04  0:02         ` Pierre Habouzit
2008-11-04  0:44           ` Junio C Hamano
2008-11-04  5:20         ` Jeff King

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=51419b2c0811011226p3369b7f7t37b98032ca8dd9ac@mail.gmail.com \
    --to=newren@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sam@vilain.net \
    --cc=samv@vilain.net \
    --cc=tytso@mit.edu \
    /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).