Re: Feature request - discard hunk in add --patch mode

2012-08-22 Thread Mina Almasry
Well I actually didn't know about stash -p when I asked for this, but I
still see room for us removing some more friction here:

 # start from N-commit worth of change, debug and WIP
 git stash save -p debug ;# stash away only the debugging aid
 # now we have only N-commit worth of change and WIP
 git stash save -p wip ;# stash away WIP
 
 Then after that, you need N round of git add -p  git commit.
 
 Now, with what we have already, can we also give final testing
 before committing?  Each round may now start with:
 
 git add -p ;# prepare the index for the next commit
 git stash save -k ;# save away the changes for later commits
 
Here you have to _at least_ go through all your hunks once to stash the
debug code, once to stash the WIP, and once to add -p and commit.

Going through hunks is expensive... it takes quite a bit of time. Each
time you launch one of those commands you have to go through all hunks
you haven't decided on yet - could be a lot of hunks.

Now, what if we had a command where you would go through all your hunks
one, by one, and will give you an option to stage, stash, or discard
what you want as you please? In the above scenario, lets say N was 1,
then the scenario would be:

# start from 1-commit worth of change, debug and WIP

# stash debugging and WIP code, 
# and stage the code you want to commit
git mystery-command

# everything done
git commit

Then we would reduce 3 -p commands to 1 -p command.

Now what should this command be called? You resist making git-add unsafe
and I understand that. How about: 

  1. Give git add -p options that expose this extra functionality. So:

git add -p ; # can only stage code
git add -p --with-discard ; # does git add -p, plus discards hunks
git add -p --with-discard --with-stage ; # plus discard, plus stage

So in the hunks you can go:
  - discard; # and it would discard it
  - stage stash; # and it would stash it into stash

The argument I am making is that if a programmer explicitly asks for one
of those features then git will cooperate, and add becomes unsafe. We
are trusting that the programmer knows what they are doing. In all other
cases git add is the same, and will never touch the working tree.

We can also change git checkout -p and git stash -p so that they behave
the same:

  git stash -p --with-add; # does everything -p stash, and stages too
  git checkout -p --with-add; # does everything -p checkout, and stages

  2. We can come up with a whole new git command that does this.
Something like git patch-process

  # start from 1-commit worth of change, debug and WIP

  git patch-process ; # Stage, stash, and discard hunks as you please

  # ready for commit
  git commit

How does this sound?

PS: Newbish question: I am looking to help out with git, either with
this feature or others. How do I get around getting assigned a work to
do? Thanks.

Mina

On Wed, 2012-08-15 at 16:58 -0400, Mina Almasry wrote:
 On 12-08-15 02:46 PM, Junio C Hamano wrote:
  Junio C Hamano gits...@pobox.com writes:
 
  Thomas Rast tr...@student.ethz.ch writes:
 
  This has come up before, and actually led to the introduction of
  'checkout -p' and 'reset -p':
 
 http://thread.gmane.org/gmane.comp.version-control.git/123854
  That is a blast from the past.
 
  Why is saying git checkout . too much work, after add -p that
  you excluded the debugging cruft?
  Please forget this question.  A better way in the form of stash -p
  was suggested in the old thread to get rid of debug cruft in the
  tree before an add -p session (or during a series of add -p
  sessions).
 
  So is this still an issue?
 
 
 I read most of the thread, and I do think it still is. Here are my 2 cents:
 
 1. The alternative commands aren't nearly as time efficient:
  - git checkout . is fast and awesome, but you can't use it if, for 
 example, you have to maintain a dirty working tree
  - git (stash|reset|checkout) -p make you go through (all|most) of 
 the hunks you have to hunt down those 2 lines that say echo 
 'This line is runningantoeuhaoeuaoae'
 
 2. All of the safety concerns can be alleviated with the right 
 interface. I am glad the u option mentioned in the thread did not go 
 through since I agree it is not ideal. However, if the command is:
  (a) something with a '!', then no one will hit it by mistake, and
  (b) the prompt makes it clear that work is lost
 then I think it would be fine
 
 The advantages of a command like this are pretty huge IMO. I can see 
 this being a big time saver.
 
 How about adding this to the git add -p prompt:
 
  r! - remove this hunk. The hunk is discarded from the working tree, 
 and is irrevocably lost.
 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 


--
To unsubscribe from this list: send the line 

Re: Feature request - discard hunk in add --patch mode

2012-08-15 Thread Thomas Rast
Mina Almasry almasry.m...@hotmail.com writes:

 I frequently stage files using git add --patch command and I almost 
 always come across debug code I want to discard, but there is no option 
 for that in the prompt. The result is that I have to run an extra 
 command after the dialogue ends.

 I would like to add a feature to allow users to discard hunks using a 
 command like r! or d!

This has come up before, and actually led to the introduction of
'checkout -p' and 'reset -p':

  http://thread.gmane.org/gmane.comp.version-control.git/123854

Judging by the '!' above, you are already aware that this is a dangerous
option that needs some safeguards.  I imagine that would largely account
for Junio's safety concerns.  So you could pick up from the general
direction of Pierre's post, and try to work out something.

However, life has become rather more complicated since 2009.  Whatever
you do also needs to fit nicely with checkout/reset/stash -p.  The first
two also take commit arguments, resulting in a total of seven modes of
operation, defined in %patch_modes.

I imagine one good angle of attack would be to proceed as follows:

* Optionally make add--interactive.perl more aware of what the patches
  say.  Ideally there would be a way to reverse the direction of a diff
  (or otherwise manipulate it) in the program itself, instead of having
  to decide this when fetching the patches.

* Change things so that the actions in %patch_modes are a hunk property
  instead of a global state.  Of course the actual mode still needs to
  be global state.  Not all actions should be possible in all modes;
  figure out a clean way to implement this.

* Design commands in some modes that switch to another mode, and/or set
  another action than the default in the current mode.

Note that r! commands need to also work in singlekey mode, where the 'r'
is read immediately and you would have to read for another
(confirmation) key to get the '!'.

Good luck!

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request - discard hunk in add --patch mode

2012-08-15 Thread Junio C Hamano
Thomas Rast tr...@student.ethz.ch writes:

 Mina Almasry almasry.m...@hotmail.com writes:

 I frequently stage files using git add --patch command and I almost 
 always come across debug code I want to discard, but there is no option 
 for that in the prompt. The result is that I have to run an extra 
 command after the dialogue ends.

 I would like to add a feature to allow users to discard hunks using a 
 command like r! or d!

 This has come up before, and actually led to the introduction of
 'checkout -p' and 'reset -p':

   http://thread.gmane.org/gmane.comp.version-control.git/123854

That is a blast from the past.

Why is saying git checkout . too much work, after add -p that
you excluded the debugging cruft?

I actually do this for extra safety, though:

git add -p ;# add everything but exclude debug cruft
git diff ;# make really sure that this shows only garbage
git diff | git apply -R ;# and get rid of that garbage

primarily because I can take the git diff output in the second
step to a file, remove the hunk that I accidentally forgot to add
in the add -p stage, and apply -R to remove only the cruft.
After doing so, git diff will show the important-but-forgotten
bit, and I can choose to add it to the next commit, or I can choose
to leave it in the working tree for a future commit after the
current index is committed.

But the above is a tangent side-note to show possibly a better way
to work, not about adding new operations to add -p.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request - discard hunk in add --patch mode

2012-08-15 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Thomas Rast tr...@student.ethz.ch writes:

 This has come up before, and actually led to the introduction of
 'checkout -p' and 'reset -p':

   http://thread.gmane.org/gmane.comp.version-control.git/123854

 That is a blast from the past.

 Why is saying git checkout . too much work, after add -p that
 you excluded the debugging cruft?

Please forget this question.  A better way in the form of stash -p
was suggested in the old thread to get rid of debug cruft in the
tree before an add -p session (or during a series of add -p
sessions).

So is this still an issue?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request - discard hunk in add --patch mode

2012-08-15 Thread Mina Almasry

On 12-08-15 02:46 PM, Junio C Hamano wrote:

Junio C Hamano gits...@pobox.com writes:


Thomas Rast tr...@student.ethz.ch writes:


This has come up before, and actually led to the introduction of
'checkout -p' and 'reset -p':

   http://thread.gmane.org/gmane.comp.version-control.git/123854

That is a blast from the past.

Why is saying git checkout . too much work, after add -p that
you excluded the debugging cruft?

Please forget this question.  A better way in the form of stash -p
was suggested in the old thread to get rid of debug cruft in the
tree before an add -p session (or during a series of add -p
sessions).

So is this still an issue?



I read most of the thread, and I do think it still is. Here are my 2 cents:

1. The alternative commands aren't nearly as time efficient:
- git checkout . is fast and awesome, but you can't use it if, for 
example, you have to maintain a dirty working tree
- git (stash|reset|checkout) -p make you go through (all|most) of 
the hunks you have to hunt down those 2 lines that say echo 
'This line is runningantoeuhaoeuaoae'


2. All of the safety concerns can be alleviated with the right 
interface. I am glad the u option mentioned in the thread did not go 
through since I agree it is not ideal. However, if the command is:

(a) something with a '!', then no one will hit it by mistake, and
(b) the prompt makes it clear that work is lost
then I think it would be fine

The advantages of a command like this are pretty huge IMO. I can see 
this being a big time saver.


How about adding this to the git add -p prompt:

r! - remove this hunk. The hunk is discarded from the working tree, 
and is irrevocably lost.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Feature request - discard hunk in add --patch mode

2012-08-15 Thread Junio C Hamano
Mina Almasry almasry.m...@hotmail.com writes:

 On 12-08-15 02:46 PM, Junio C Hamano wrote:
 ...
 Please forget this question.  A better way in the form of stash -p
 was suggested in the old thread to get rid of debug cruft in the
 tree before an add -p session (or during a series of add -p
 sessions).

 So is this still an issue?

 I read most of the thread, and I do think it still is. Here are my 2 cents:

 1. The alternative commands aren't nearly as time efficient:
 - git checkout . is fast and awesome, but you can't use it if, for
 example, you have to maintain a dirty working tree
 - git (stash|reset|checkout) -p make you go through (all|most) of
 the hunks you have to hunt down those 2 lines that say
 echo 'This line is runningantoeuhaoeuaoae'

You have to do that _at least once_ anyway, as there is no other way
for Git to tell which one is debugging cruft and which one is the
real change you value without you telling it.  Will return to the
topic later.

 2. All of the safety concerns can be alleviated with the right
 interface.

There are three safety concerns I raised in the original thread.
Among them, I do not think

 (1) new user may mistake undo to be something safe; and
 (2) experts will continue making typo between y and u

are the primary risks of the original patch Thomas pointed out.  

A much bigger problem with the approach is (3) letting add touch
the working tree breaks the world model.  Both experts and newbies
alike, people have learned that git add will never clobber what
they have in the working tree and rely on that promise.

And your key assignment, command renaming or extra prompting do not
change this fundamental issue at all.

Let's step back a bit, and define the problem we are solving.

Suppose you have changes in your working tree that are worth
multiple commits, debugging aid, and uncommittable WIP.  You want to
create multiple commits, possibly giving each of them the final
testing before committing, and want to end up with the WIP (plus
possibly the debugging aid, as that may still help your WIP) in your
working tree.

Do we agree that the goal of the discussion of this thread is to
make that process simple, safe, efficient and easy?

Now, back when the original patch by Pierre was proposed, it indeed
was cumbersome.  You could sift things through by add -p to build
the first commit in the index, commit, and iterate.  In each round,
add -p step had to skip the same debugging aid and WIP over and
over again.  If you wanted to give the result of git add -p a
final test before committing, stash save -k would give you the
state you would be committing, but it isn't easy to reintroduce only
the debugging aid to the working tree.

Since then stash -p was added to our toolchest.  So theoretically,
we should be able to do something like this:

# start from N-commit worth of change, debug and WIP
git stash save -p debug ;# stash away only the debugging aid
# now we have only N-commit worth of change and WIP
git stash save -p wip ;# stash away WIP

Then after that, you need N round of git add -p  git commit.

Now, with what we have already, can we also give final testing
before committing?  Each round may now start with:

git add -p ;# prepare the index for the next commit
git stash save -k ;# save away the changes for later commits

and at this point, your working tree and the index has what you are
about to commit.  If we can apply the debugging aid stash we
created earlier to the working tree, that would allow you to test
the state you are about to commit with your debugging aid.  The
command to do so may be

git stash apply stash@{2}

Once you are satisfied, you can reset this change out of your
working tree with checkout ., and then git commit to record what
is in the index.

And then you can git stash pop to bring back the remainder of
N-commit series worth of changes.  You are ready to continue to the
next round.

Once you created N-commit series, you will still have two stash
entries, one debug and one wip.  You should be able to resurrect
wip with

git stash pop

just fine, but there is a little problem after this step.  Because
git stash [apply|pop] does not want to work on a dirty working
tree, starting from this state just after popping wip stash, you
cannot git stash pop to have both WIP changes and debugging aid to
the working tree.

A topic to improve stash [apply/pop] to allow it may be a valid
and useful thing to do.

As an approximation, without changing any of the current tools,
however, you should be able to do this after creating N-commit
series following the above procedure.

git stash pop ;# resurrect wip to the working tree.
git add -u ;# and add that to the index temporarily
git stash pop ;# resurrect debug to the working tree as well.
git reset ;# then match the index to HEAD

That will mix the WIP and debug again in your working tree.

Why you would want to mix