Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

6 messages, 5 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:47

Junio C Hamano [off-list ref] writes:
Johannes Schindelin [off-list ref] writes:
quoted
In the same way, I would expect "git revert <commit> -- file" to undo the 
changes in that commit to _that_ file (something like "git merge-file 
file <commit>:file <commit>^:file"), but this time commit it, since it 
was committed at one stage.
Allowing people to revert or cherry pick partially by using
paths limiter is a very good idea; ...
As Pierre said earlier, a partial revert via "revert <commit> --
<paths>" and a partial cherry-pick would make quite a lot of
sense, and in addition, it should not be too hard to add.

Reusing the 'merge-recursive' part should be almost trivial.
The only tricky part is coming up with a fake tree using base
and next commit in revert_or_cherry_pick() for this purpose.

When replaying the change from A->B (when cherry-picking, A is
the parent and B is what was named from the command line; when
reverting, they are the other way around), instead of doing the
three-way merge using:

	merge-recursive A HEAD B

you would first come up with a modified tree B' that has the
identical contents to A _except_ the parts the path limiters
specify which are taken from B.  Then running

	merge-recursive A HEAD B'

would replay the revert or cherry-pick of change from A->B,
limited by the path, on top of the current HEAD.

As to "reverting to the index" case, if somebody is interested
in doing a builtin-checkout.c, please keep in mind that major
parts of that work should be made available to the
implementation of "git revert [--] <paths>", as it appears that
it will be exactly the same as "git checkout" with the same set
of options.

I am wondering what "git cherry-pick -- <paths>" should do.  My
current thinking is that it would not make any sense at all.

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:43:47

Junio C Hamano schrieb:
I am wondering what "git cherry-pick -- <paths>" should do.  My
current thinking is that it would not make any sense at all.
IMO, at least "git cherry-pick -n -- <paths>" makes tons of sense.

-- Hannes

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:47

Hi,

On Tue, 6 Nov 2007, Johannes Sixt wrote:
Junio C Hamano schrieb:
quoted
I am wondering what "git cherry-pick -- <paths>" should do.  My
current thinking is that it would not make any sense at all.
IMO, at least "git cherry-pick -n -- <paths>" makes tons of sense.
I guess you missed that Junio did not specify any commit.  With a commit, 
I agree, it makes tons of sense.

Without a commit, it would default to... uhm... HEAD?  And applying the 
changes to a given file, which are already in HEAD, no, that does not make 
sense.

Ciao,
Dscho

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:47

Hi,

On Tue, 6 Nov 2007, Junio C Hamano wrote:
Junio C Hamano [off-list ref] writes:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
In the same way, I would expect "git revert <commit> -- file" to undo the 
changes in that commit to _that_ file (something like "git merge-file 
file <commit>:file <commit>^:file"), but this time commit it, since it 
was committed at one stage.
Allowing people to revert or cherry pick partially by using
paths limiter is a very good idea; ...
As Pierre said earlier, a partial revert via "revert <commit> --
<paths>" and a partial cherry-pick would make quite a lot of
sense, and in addition, it should not be too hard to add.
Yes, but Pierre also said earlier that people want to revert their local 
changes.  And the logical thing to try that really is

	git revert <path>

Now, if you read that out in English, it does not make too much sense: 
"revert the path" (not "revert the _changes_ to that file").  But it is 
what people try to do.

However, IIUC another thing Pierre mentioned is that

	$scm revert <commit> <path>

commonly means "revert the file _to the version_ stored in <commit>".  
This is just different enough from "revert the _changes_ to that file 
stored in <commit>" to bite people, no?
Reusing the 'merge-recursive' part should be almost trivial. The only 
tricky part is coming up with a fake tree using base and next commit in 
revert_or_cherry_pick() for this purpose.
FWIW I really wanted to use the merge-file machinery, not the 
merge-recursive one.  But since "<path>" can be a directory, too, I was 
mistaken, and you are correct, as always.
As to "reverting to the index" case, if somebody is interested in doing 
a builtin-checkout.c, please keep in mind that major parts of that work 
should be made available to the implementation of "git revert [--] 
<paths>", as it appears that it will be exactly the same as "git 
checkout" with the same set of options.
I was planning to put cmd_checkout() into builtin-reset.c for that reason.

But first things first, that "git remote prune" with --mirror'ed 
repositories misbehaviour annoys me just enough that I started converting 
this script first.  It has been stable enough for quite a long time, and 
the script now shows its limitations.

Besides, remote.[ch] makes it easy, even if not _really_ easy.

Ciao,
Dscho

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Pierre Habouzit <hidden>
Date: 2016-06-15 22:43:47

On Tue, Nov 06, 2007 at 12:25:33PM +0000, Johannes Schindelin wrote:
Hi,

On Tue, 6 Nov 2007, Junio C Hamano wrote:
quoted
Junio C Hamano [off-list ref] writes:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
In the same way, I would expect "git revert <commit> -- file" to undo the
changes in that commit to _that_ file (something like "git merge-file
file <commit>:file <commit>^:file"), but this time commit it, since it
was committed at one stage.
Allowing people to revert or cherry pick partially by using
paths limiter is a very good idea; ...
As Pierre said earlier, a partial revert via "revert <commit> --
<paths>" and a partial cherry-pick would make quite a lot of
sense, and in addition, it should not be too hard to add.
Yes, but Pierre also said earlier that people want to revert their local
changes.  And the logical thing to try that really is

	git revert <path>

Now, if you read that out in English, it does not make too much sense:
"revert the path" (not "revert the _changes_ to that file").  But it is
what people try to do.

However, IIUC another thing Pierre mentioned is that

	$scm revert <commit> <path>

commonly means "revert the file _to the version_ stored in <commit>".
This is just different enough from "revert the _changes_ to that file
stored in <commit>" to bite people, no?
  Yeah but that's what checkout is for. The main source of iritation for
new users comes (IMHO) from svn, where `svn revert path/to/file` is part
of the workflow: in case of a conflict when you `svn up`, you have
either to:
  (1) fix the conflict and `svn resolved path/to/file`
  (2) drop your changes and take the trunk version `svn revert path/to/file`

People really expect git revert -- path/to/file to do the same as git
checkout HEAD -- path/to/file. Though I believe that like I said, maybe
we don't wan't git revert -- path/to/file to become the first class
command to do that, but rather to do what the user meant, hinting him in
the direction of the proper command. I wasn't really advocating that
git-revert should be a complete implementation of what git checkout
<comitish> -- <paths> does. YMMV.

--
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Re: [PATCH] git-revert is one of the most misunderstood command in git, help users out.

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:43:47

El 6/11/2007, a las 13:48, Pierre Habouzit escribió:
On Tue, Nov 06, 2007 at 12:25:33PM +0000, Johannes Schindelin wrote:
quoted
Hi,

On Tue, 6 Nov 2007, Junio C Hamano wrote:
quoted
Junio C Hamano [off-list ref] writes:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
In the same way, I would expect "git revert <commit> -- file" to  
undo the
changes in that commit to _that_ file (something like "git merge- 
file
file <commit>:file <commit>^:file"), but this time commit it,  
since it
was committed at one stage.
Allowing people to revert or cherry pick partially by using
paths limiter is a very good idea; ...
As Pierre said earlier, a partial revert via "revert <commit> --
<paths>" and a partial cherry-pick would make quite a lot of
sense, and in addition, it should not be too hard to add.
Yes, but Pierre also said earlier that people want to revert their  
local
changes.  And the logical thing to try that really is

	git revert <path>

Now, if you read that out in English, it does not make too much  
sense:
"revert the path" (not "revert the _changes_ to that file").  But  
it is
what people try to do.

However, IIUC another thing Pierre mentioned is that

	$scm revert <commit> <path>

commonly means "revert the file _to the version_ stored in <commit>".
This is just different enough from "revert the _changes_ to that file
stored in <commit>" to bite people, no?
 Yeah but that's what checkout is for. The main source of iritation  
for
new users comes (IMHO) from svn, where `svn revert path/to/file` is  
part
of the workflow: in case of a conflict when you `svn up`, you have
either to:
 (1) fix the conflict and `svn resolved path/to/file`
 (2) drop your changes and take the trunk version `svn revert path/ 
to/file`

People really expect git revert -- path/to/file to do the same as git
checkout HEAD -- path/to/file. Though I believe that like I said,  
maybe
we don't wan't git revert -- path/to/file to become the first class
command to do that, but rather to do what the user meant, hinting  
him in
the direction of the proper command. I wasn't really advocating that
git-revert should be a complete implementation of what git checkout
<comitish> -- <paths> does. YMMV.

I agree; they're semantically different and it wouldn't be a good  
thing to start blurring the lines between them too much. It's just  
unfortunate that the term "revert" is used by most other SCMs to mean  
something different than what it means in "git-revert". I think the  
best path here is education, what Pierre says, rather than changing  
git-revert's semantics.

The other changes discussed so far in this thread (path-limiting git- 
revert with preserving its semantics) seem like a good thing.

Cheers,
Wincent
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help