[PATCH] update-index: add --swap to swap index and worktree content

[PATCH] update-index: add --swap to swap index and worktree content

[Nguyễn Thái Ngọc Duy]

Sometimes "git add -p" with "e" to edit the patch does not satisfy me.
What I want is a quick way to modify index content without changing
worktree, then I can continue adding more hunks to the index.

With this option, I can swap index out for a quick edit, then swap it in
again.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 Not sure if anybody has the same needs, enough to polish it and make
 it to master.

 builtin/update-index.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index a6a23fa..b96065a 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -22,6 +22,7 @@
 static int allow_add;
 static int allow_remove;
 static int allow_replace;
+static int swap_entry;
 static int info_only;
 static int force_remove;
 static int verbose;
@@ -104,6 +105,19 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru
 		free(ce);
 		return -1;
 	}
+	if (swap_entry) {
+		struct checkout state;
+		if (allow_add || allow_remove)
+			die("--add, --replace and --swap do not play together");
+		memset(&state, 0, sizeof(state));
+		state.force = 1;
+		state.not_new = 1;
+		if (add_cache_entry(ce, 0))
+			return error("%s: cannot add to the index", path);
+		if (checkout_entry(old, &state, NULL))
+			return error("%s: cannot swap", path);
+		return 0;
+	}
 	option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
 	option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
 	if (add_cache_entry(ce, option))
@@ -727,6 +741,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			"let files replace directories and vice-versa", 1),
 		OPT_SET_INT(0, "remove", &allow_remove,
 			"notice files missing from worktree", 1),
+		OPT_SET_INT(0, "swap", &swap_entry,
+			"swap the content of index and worktree", 1),
 		OPT_BIT(0, "unmerged", &refresh_args.flags,
 			"refresh even if index contains unmerged entries",
 			REFRESH_UNMERGED),
-- 
1.7.4.74.g639db

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Michael J Gruber]

Nguyễn Thái Ngọc Duy venit, vidit, dixit 12.08.2011 16:07:
> Sometimes "git add -p" with "e" to edit the patch does not satisfy me.
> What I want is a quick way to modify index content without changing
> worktree, then I can continue adding more hunks to the index.
> 
> With this option, I can swap index out for a quick edit, then swap it in
> again.

I had to think about that explanation for a while (partly because "git
add" does not alter the wt either). So, your patch would support the
following workflow:

git add foo # have index == wt
hack foo # change foo in wt
git update-index --swap foo # add foo and reset to previous state

Am I understanding you right? The option could trickle down to "add".

I share the pov that "add -p with e" sometimes doesn't cut it. But
similarly, the fact that "add -p" can't be used to undo a previous "add
-p" is suboptimal. Both issues could be solved with a 3way stage tool. I
have this on my todo/wish list, and I seem to recall that Jeff or Junio
came up with a few lines of (scripting) code for that. That would depend
on the availability of proper tools, though (e.g. vim in diff mode).

--swap might be useful nevertheless.

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  Not sure if anybody has the same needs, enough to polish it and make
>  it to master.
> 
>  builtin/update-index.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index a6a23fa..b96065a 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -22,6 +22,7 @@
>  static int allow_add;
>  static int allow_remove;
>  static int allow_replace;
> +static int swap_entry;
>  static int info_only;
>  static int force_remove;
>  static int verbose;
> @@ -104,6 +105,19 @@ static int add_one_path(struct cache_entry *old, const char *path, int len, stru
>  		free(ce);
>  		return -1;
>  	}
> +	if (swap_entry) {
> +		struct checkout state;
> +		if (allow_add || allow_remove)
> +			die("--add, --replace and --swap do not play together");
> +		memset(&state, 0, sizeof(state));
> +		state.force = 1;
> +		state.not_new = 1;
> +		if (add_cache_entry(ce, 0))
> +			return error("%s: cannot add to the index", path);
> +		if (checkout_entry(old, &state, NULL))
> +			return error("%s: cannot swap", path);
> +		return 0;
> +	}
>  	option = allow_add ? ADD_CACHE_OK_TO_ADD : 0;
>  	option |= allow_replace ? ADD_CACHE_OK_TO_REPLACE : 0;
>  	if (add_cache_entry(ce, option))
> @@ -727,6 +741,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  			"let files replace directories and vice-versa", 1),
>  		OPT_SET_INT(0, "remove", &allow_remove,
>  			"notice files missing from worktree", 1),
> +		OPT_SET_INT(0, "swap", &swap_entry,
> +			"swap the content of index and worktree", 1),
>  		OPT_BIT(0, "unmerged", &refresh_args.flags,
>  			"refresh even if index contains unmerged entries",
>  			REFRESH_UNMERGED),

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Nguyen Thai Ngoc Duy]

2011/8/16 Michael J Gruber <git@drmicha.warpmail.net>:
> I had to think about that explanation for a while (partly because "git
> add" does not alter the wt either). So, your patch would support the
> following workflow:
>
> git add foo # have index == wt
> hack foo # change foo in wt
> git update-index --swap foo # add foo and reset to previous state
>
> Am I understanding you right? The option could trickle down to "add".

Well, the point is I want to skip the first phase of normal edit path
(wt -> index -> commit) and edit index directly (even in parts that
are not touched by wt, which means "git add -e" is useless), leaving
my wt "unchanged". Sometimes wt is just too dirty I don't want to make
any more changes to it while I focus on "index -> commit" phase.

I could add an option to fire up editor with a temporary file
containing current index content and automatically update index after
editor is closed. But I'd lose the ability to diff my changes. So I do
need to wt to change index without losing my current wt. I have to
stash my wt somewhere, and index seems a safe place for that.

After swapping, it'd look like index -> wt -> commit, I can do "diff
HEAD" to check index vs head. Once I'm done (and create index'), I can
swap it back to wt -> index' -> commit.

Thinking a bit more, this may have another use case (for lazy guys
like me, anyway). Assume I have separate changes in wt and I'm ready
to make many commit. I should make good commits, which means tests (or
at least a compile to see I don't miss any changes). So I add related
changes to index, then swap wt/index so index becomes wt (and old wt
hidden in index). Test what is to be committed. Good? Swap back,
commit. Repeat with the rest of changes in wt.

My current workflow is picking changes I believe should go together,
commit, then "rebase -i", stopping at each commit to test. It takes
longer (especially when I mis-pick changes).

> I share the pov that "add -p with e" sometimes doesn't cut it. But
> similarly, the fact that "add -p" can't be used to undo a previous "add
> -p" is suboptimal. Both issues could be solved with a 3way stage tool. I
> have this on my todo/wish list, and I seem to recall that Jeff or Junio
> came up with a few lines of (scripting) code for that. That would depend
> on the availability of proper tools, though (e.g. vim in diff mode).

That'd be great.
-- 
Duy

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> What I want is a quick way to modify index content without changing
> worktree, then I can continue adding more hunks to the index.

Why would you even want to do that?

Suppose you want to update hello.c in the index but not in the working
tree for whatever reason I do not understand.  Presumably you would with
this patch do this:

	edit hello.c
        git update-index --swap hello.c

assuming that the state of hello.c _before_ the edit was pristine.  But
then after that what would you do?  Probably you would commit that
untested change, and rebase-i to clean them up later, which is fine.

BUT

After swapping, the file in the working tree lacks the change you made to
the index, so if you keep working on it after the above "swap" in your
editor, the next "git add" will revert whatever you did with the first
"edit".

I think that makes it too error prone and difficult to understand for both
new and experienced people.

Whatever the expected use case is, it needs to be explained a lot better.

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Jeff King]

On Tue, Aug 16, 2011 at 01:01:41PM -0700, Junio C Hamano wrote:

> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> 
> > What I want is a quick way to modify index content without changing
> > worktree, then I can continue adding more hunks to the index.
> 
> Why would you even want to do that?
> 
> Suppose you want to update hello.c in the index but not in the working
> tree for whatever reason I do not understand.  Presumably you would with
> this patch do this:
> 
> 	edit hello.c
>         git update-index --swap hello.c
> 
> assuming that the state of hello.c _before_ the edit was pristine.  But
> then after that what would you do?  Probably you would commit that
> untested change, and rebase-i to clean them up later, which is fine.

I suspect (or at least, this is how I would use it) it is more about
having some changes in the index and some changes in the working tree,
but realizing that what's in the index needs tweaked. Something like:

  # add some content with an error
  echo 'printf("hello word!\n")' >hello.c
  git add hello.c

  # work on it more, realizing the error
  echo 'printf("goodbye world!\n") >hello.c

  # now what? you want to stage the s/word/world/ fixup,
  # but you want to keep the hello/goodbye thing as a separate change.
  # Using anything line-based is going to conflate the two.
  # The change is simple, though, so you can just as easily edit the
  # index file, if only you could get to it. So you do:
  git update-index --swap hello.c
  sed -i s/word/world/ hello.c
  git update-index --swap hello.c

So the swap really functions as a toggle of "I would like to work on
the index version for a minute", and then you toggle back when you're
done.

I can think of two ways to do the same thing that are a little less
confusing or error-prone, though:

  1. A command to dump the index version to a tempfile, run $EDITOR on
     it, and then read it back in. Technically this restricts you to
     using $EDITOR to make the changes, but in practice that is probably
     fine.

  2. You can dump the file to a pipe yourself, but getting it back into
     the index is a little bit awkward. You have to do something like:

       blob=`git cat-file blob :hello.c |
             sed 's/word/world/ |
             git hash-object --stdin -w`
       mode=`git ls-files -s hello.c | cut -d' ' -f1`
       git update-index --cacheinfo $mode $blob hello.c

     it would be much nicer if this was:

       git cat-file blob :hello.c |
       sed 's/word/world/ |
       git add --stdin-contents hello.c

     However, I expect this sort of piping is the minority case, and
     most people would be happy with (1).

-Peff

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>   # add some content with an error
>   echo 'printf("hello word!\n")' >hello.c
>   git add hello.c
>
>   # work on it more, realizing the error
>   echo 'printf("goodbye world!\n") >hello.c
>
>   # now what? you want to stage the s/word/world/ fixup,
>   # but you want to keep the hello/goodbye thing as a separate change.
>   # Using anything line-based is going to conflate the two.
>   # The change is simple, though, so you can just as easily edit the
>   # index file, if only you could get to it. So you do:
>   git update-index --swap hello.c
>   sed -i s/word/world/ hello.c
>   git update-index --swap hello.c
>
> So the swap really functions as a toggle of "I would like to work on
> the index version for a minute", and then you toggle back when you're
> done.

And you have to redo what you did to the index version in the working tree
after the second "swap", no?

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Jeff King]

On Tue, Aug 16, 2011 at 02:56:41PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> >   # add some content with an error
> >   echo 'printf("hello word!\n")' >hello.c
> >   git add hello.c
> >
> >   # work on it more, realizing the error
> >   echo 'printf("goodbye world!\n") >hello.c
> >
> >   # now what? you want to stage the s/word/world/ fixup,
> >   # but you want to keep the hello/goodbye thing as a separate change.
> >   # Using anything line-based is going to conflate the two.
> >   # The change is simple, though, so you can just as easily edit the
> >   # index file, if only you could get to it. So you do:
> >   git update-index --swap hello.c
> >   sed -i s/word/world/ hello.c
> >   git update-index --swap hello.c
> >
> > So the swap really functions as a toggle of "I would like to work on
> > the index version for a minute", and then you toggle back when you're
> > done.
> 
> And you have to redo what you did to the index version in the working tree
> after the second "swap", no?

No. The point is that I _already_ did it in the working tree version
while doing my s/hello/goodbye/ change (let's call this the "new
change"). And ideally I would just use "git add -p" to stage only the
s/word/world/ change (let's call this the "fixup"). But they're tangled
in a single hunk, and I need some way of splitting them.

You could also use "add -e" to solve this situation. In this case, you
end up removing the "new change" from the patch, leaving only the fixup.
In this toy case, it would be pretty easy. But in more complex cases,
the fixup is a one-liner, but the "new change" is many lines. So rather
than edit out the new changes, it is simpler to recreate the fixup on
top of the index version.

-Peff

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> > So the swap really functions as a toggle of "I would like to work on
>> > the index version for a minute", and then you toggle back when you're
>> > done.
>> 
>> And you have to redo what you did to the index version in the working tree
>> after the second "swap", no?
>
> No. The point is that I _already_ did it in the working tree version

But that does not change that you have to do that twice. You may already
have done so in the working tree, and then redo it in the old indexed
version again.

> while doing my s/hello/goodbye/ change (let's call this the "new
> change"). And ideally I would just use "git add -p" to stage only the
> s/word/world/ change (let's call this the "fixup"). But they're tangled
> in a single hunk, and I need some way of splitting them.

As a way to punt from making "add -e" usable, I'd think it would be a
workable q&d workaround, even though it feels wrong, and I would imagine
that normal people would probably prefer the "check out to a temporary
file to be edited" solution you wrote in your previous message.

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Jeff King]

On Tue, Aug 16, 2011 at 04:01:57PM -0700, Junio C Hamano wrote:

> > No. The point is that I _already_ did it in the working tree version
> 
> But that does not change that you have to do that twice. You may already
> have done so in the working tree, and then redo it in the old indexed
> version again.

Right. The point is that the changes are tangled, to the point that it
is simpler to recreate them on one side than it is to instruct a tool
about how to untangle them. If they weren't tangled, then I would simply
make the change in the working tree, and then "add -p" it into the
index.

It's certainly not the dominant case. I'd say I run into it no more
frequently than once every week or so. I usually end up using "add -e"
to do what I want, but it's very error prone and annoying.

> > while doing my s/hello/goodbye/ change (let's call this the "new
> > change"). And ideally I would just use "git add -p" to stage only the
> > s/word/world/ change (let's call this the "fixup"). But they're tangled
> > in a single hunk, and I need some way of splitting them.
> 
> As a way to punt from making "add -e" usable, I'd think it would be a
> workable q&d workaround, even though it feels wrong, and I would imagine
> that normal people would probably prefer the "check out to a temporary
> file to be edited" solution you wrote in your previous message.

Yeah, I think that is the sanest of the options brought up in this
thread. I'm curious if Duy had another use case, though, that made him
think of --swap.

-Peff

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Nguyen Thai Ngoc Duy]

2011/8/17 Jeff King <peff@peff.net>:
>> As a way to punt from making "add -e" usable, I'd think it would be a
>> workable q&d workaround, even though it feels wrong, and I would imagine
>> that normal people would probably prefer the "check out to a temporary
>> file to be edited" solution you wrote in your previous message.
>
> Yeah, I think that is the sanest of the options brought up in this

I blame git for training me always do "git diff" before update index.
But if we add "add -e", I could also make "add -e -p" work, (edit
temporary file first, then fall back to interactive mode and let me
review my changes).

> thread. I'm curious if Duy had another use case, though, that made him
> think of --swap.

Two reasons. I already mentioned the ability to quickly checkout index
for a quick test (though there could be more problems down that road).

The other one could be done another way too. I translation
coordinator. When a translator sends their translations to me, I'd
pick good translations and commit them, leaving the rest for
discussion. This is why I don't want to touch wt. While adding
translations to index, I may find a mistake in current translation
(ie. in index) and want to fix it too.
-- 
Duy

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Jeff King]

On Wed, Aug 17, 2011 at 09:11:32AM +0700, Nguyen Thai Ngoc Duy wrote:

> I blame git for training me always do "git diff" before update index.
> But if we add "add -e", I could also make "add -e -p" work, (edit
> temporary file first, then fall back to interactive mode and let me
> review my changes).

Yeah, that would be cool. But note that "git add -e" already exists for
patch editing.

> Two reasons. I already mentioned the ability to quickly checkout index
> for a quick test (though there could be more problems down that road).

That's a good thought. However, in practice, I find I also need other
files from the index to do a successful test. So I end up just
committing what I think is right, and then afterwards do:

  GIT_EDITOR='sed -i "/^pick .*/aexec make test/"' git rebase -i

to test and fixup each commit individually.

You could probably use "checkout-index" to create a pristine workspace
to test. It's a little inefficient to write out all of the files again,
but probably the actual build and test procedure would be much more
expensive, anyway.

-Peff

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Martin von Zweigbergk]

Hi,

On Tue, Aug 16, 2011 at 10:17 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Aug 17, 2011 at 09:11:32AM +0700, Nguyen Thai Ngoc Duy wrote:
>
>> Two reasons. I already mentioned the ability to quickly checkout index
>> for a quick test (though there could be more problems down that road).
>
> That's a good thought. However, in practice, I find I also need other
> files from the index to do a successful test. So I end up just
> committing what I think is right, and then afterwards do:

Maybe a stupid question, but "update-index --swap" would swap all
files, right? So what "other files from the index" would there be?

Maybe the above shows that I'm missing something. Still, would it be
possible to achieve the use case with something like the following?

git stash --keep-index
# fix up files, test etc.
git add ....
git commit
git cherry-pick --strategy=theirs stash && git stash drop

Martin

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Nguyen Thai Ngoc Duy]

On Wed, Aug 17, 2011 at 9:13 PM, Martin von Zweigbergk
<martin.von.zweigbergk@gmail.com> wrote:
> Hi,
>
> On Tue, Aug 16, 2011 at 10:17 PM, Jeff King <peff@peff.net> wrote:
>> On Wed, Aug 17, 2011 at 09:11:32AM +0700, Nguyen Thai Ngoc Duy wrote:
>>
>>> Two reasons. I already mentioned the ability to quickly checkout index
>>> for a quick test (though there could be more problems down that road).
>>
>> That's a good thought. However, in practice, I find I also need other
>> files from the index to do a successful test. So I end up just
>> committing what I think is right, and then afterwards do:
>
> Maybe a stupid question, but "update-index --swap" would swap all
> files, right? So what "other files from the index" would there be?

That's some of the "problems down that road", another one being
removed files in index. Still the ability to do a quick compile on a
single file is cool.

> Maybe the above shows that I'm missing something. Still, would it be
> possible to achieve the use case with something like the following?
>
> git stash --keep-index
> # fix up files, test etc.
> git add ....
> git commit
> git cherry-pick --strategy=theirs stash && git stash drop

Pretty much (if I understand git-stash correctly, I haven't used it
much), except that if I change my mind mid way, I can switch back to
the original state and keep working:

1. git add -p ...
2. git update-index --swap foo # index <-> wt
3. # make some minor edit, "make foo.o", oops failed, I missed some hunks
4. git checkout -p ...
5. # still not happy, more changes should be made on both index and wt
6. git update-index --swap foo # now index is index, wt is wt
7. # edit some more, when ready to commit, repeat steps 2-6 again
8. git commit
-- 
Duy

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Junio C Hamano]

Martin von Zweigbergk <martin.von.zweigbergk@gmail.com> writes:

> Maybe the above shows that I'm missing something. Still, would it be
> possible to achieve the use case with something like the following?
>
> git stash --keep-index

While I sense a vague aversion to committing in general in this
discussion, which I am not particularly fond of, the whole point of
"stash" is to avoid the mental burden of going over the "hump" of
committing something not ready and to replace it with a "save it as a
temporary state" that technically has the same overhead of committing but
has a lot less mental overhead. Perhaps "swapping the state of all and/or
selected paths" fits better in "stash", not "update-index"?

I dunno, but in general, a new feature to cater to _common_ end user needs
should fit in the Porcelain layer. We would be doing something wrong if we
need to teach an obscure option of lower plumbing to end users.

I personally used to feel that "ls-files -u" during a conflicted merge to
be the single sore-thumb that stuck out from this point of view, but these
days "status -s" gives the same information in a more useful way to the
end users, and I am reasonably happy with that. It may be that the end
user operation (perhaps "stash --swap", but I am not married to that) that
fits well in common workflows ends up using "update-index --swap" as an
underlying implementation detail, but I'd prefer to see how the final end
user experience using Porcelain would look like first.

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Jeff King]

On Wed, Aug 17, 2011 at 10:13:08AM -0400, Martin von Zweigbergk wrote:

> >> Two reasons. I already mentioned the ability to quickly checkout index
> >> for a quick test (though there could be more problems down that road).
> >
> > That's a good thought. However, in practice, I find I also need other
> > files from the index to do a successful test. So I end up just
> > committing what I think is right, and then afterwards do:
> 
> Maybe a stupid question, but "update-index --swap" would swap all
> files, right? So what "other files from the index" would there be?

Hmm. I hadn't really considered swapping everything. I suppose that
would work, though I do worry about getting into a confused state when
you have swapped one or more files individually, and then want to swap
the rest out for testing.

That is, there is no flag to say "this file was swapped", so there is no
way to say "swap all of the index files out, except those I have already
swapped". But if you were disciplined to only do:

  git update-index --swap
  $EDITOR foo.c
  make test
  git update-index --swap

and never:

  git update-index --swap foo.c
  $EDITOR foo.c
  git update-index --swap ;# oops, we put the WT foo.c back
  make test
  git update-index --swap

then it would work.

-Peff

Re: [PATCH] update-index: add --swap to swap index and worktree content

[Martin von Zweigbergk]

On Wed, Aug 17, 2011 at 3:46 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Aug 17, 2011 at 10:13:08AM -0400, Martin von Zweigbergk wrote:
>
>> >> Two reasons. I already mentioned the ability to quickly checkout index
>> >> for a quick test (though there could be more problems down that road).
>> >
>> > That's a good thought. However, in practice, I find I also need other
>> > files from the index to do a successful test. So I end up just
>> > committing what I think is right, and then afterwards do:
>>
>> Maybe a stupid question, but "update-index --swap" would swap all
>> files, right? So what "other files from the index" would there be?
>
> Hmm. I hadn't really considered swapping everything.

Ah, then I see where the misunderstanding lay. I didn't read the
patch; maybe it would have been clear if I did (and understood it).

> I suppose that
> would work, though I do worry about getting into a confused state when
> you have swapped one or more files individually, and then want to swap
> the rest out for testing.

I have a feeling that I would get confused if I started swapping more
than one individual file, so to me it feels like it's more the
possibility of swapping individual files that could cause confusion,
whether or not it is possible to swap all.

I think stashing the work tree feels more right, though. All you
really want is to get the work tree version out of the way for a
while, you don't really want it in the index.

Martin