Re: [PATCH 4/4] reset: add test cases for "--merge-dirty" option

[Christian Couder <chriscool@tuxfamily.org>]

On Friday 11 September 2009, Daniel Barkalow wrote:
> On Fri, 11 Sep 2009, Christian Couder wrote:
> > On Friday 11 September 2009, Daniel Barkalow wrote:
> > > On Thu, 10 Sep 2009, Christian Couder wrote:
> > >
> > >     This shows that with the "--merge-dirty" option,
> > >
> > >   changes that are both in the work tree and the index are kept
> > >
> > >     in the work tree after the reset (but discarded in the index). As
> > > with the "--merge" option,
> > >
> > >   changes that are in both the work tree and the index are discarded
> > >
> > >     after the reset.
> > >
> > > I'm lost here.
> > >
> > > If you have:
> > >
> > >          working index HEAD target
> > > version     B      B     A     A
> > >
> > > You get:
> > >
> > >          working index HEAD target
> > > --m-d       B      A     A     A
> > > --merge     A      A     A     A
> > >
> > > ?
> >
> > Yes, files that are not different between HEAD and target are changed
> > like that. Thanks for explaining it better than I could!
>
> I worked on the rules for merging way back when, so I've looked at tables
> of cases like that. If there are more cases to cover, it might work
> better to have a table like:
>
> working index HEAD target         working index HEAD
>    B      B     A     A   --m-d      B      A     A
>                           --merge    A      A     A
>    B      B     A     C   --m-d       (disallowed)
>                           --merge    C      C     C
>
> Are there other differences?

Yes, I found that I messed up the last test in patch 4/4. I forgot to 
replace some --merge with --merge-dirty :-(

In fact while "reset --merge" fails when there are changes in files that are 
changed between HEAD and target, "reset --merge-dirty" will not fail and 
discard these changes. So it is not really safe in this case and I am 
working on trying to make it safer in this case.

> > > > ---
> > > >  t/t7110-reset-merge.sh |   54
> > > > +++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49
> > > > insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> > > > index 45714ae..1e6d634 100755
> > > > --- a/t/t7110-reset-merge.sh
> > > > +++ b/t/t7110-reset-merge.sh
> > > > @@ -19,7 +19,7 @@ test_expect_success 'creating initial files' '
> > > >       git commit -m "Initial commit"
> > > >  '
> > > >
> > > > -test_expect_success 'ok with changes in file not changed by reset'
> > > > ' +test_expect_success '--merge: ok if changes in file not touched
> > > > by reset' '
> > >
> > > Should probably have the "--merge: " from the beginning, since you're
> > > adding the test in this series anyway. That would make the diff come
> > > out clearer.
> >
> > Yeah, but I am not sure that patches 3/4 and 4/4 will get merged in the
> > end. If they are not merged it will be better if there is no "--merge:
> > ".
>
> Maybe write those lines to mention "reset --merge" naturally? Like:
>
> 'ok with changes in file not changed by reset --merge'
>
> 'reset --merge discards changes added to index 1'

Ok I will do that.

Thanks,
Christian.