Re: [PATCH] git add -e documentation: rephrase note

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Wesley J. Landaker" <wjl@xxxxxxxxxxxxx> writes:

> On Monday 19 October 2009 01:07:23 Junio C Hamano wrote:
>> That is Ok; the comment was not about stage vs add.
>>
>> > But beyond that, yes, you are right that removing a "+" line may have a
>> > different conceptual meaning to the user depending on the surrounding
>> > text. I wonder if such a "check-list" document really makes much sense,
>> > given that using "-e" at all means you need to understand the patch
>> > format and what makes sense (i.e., anybody who understands 'patch'
>> > knows that you can't just delete context lines and expect it to apply).
>>
>> Yeah, that is really what I wanted people who are in this discussion to
>> eventually realize ;-)
>
> Comment from the peanut gallery:
>
> I still think a quick summary checklist is useful even for a seasoned 
> developer that is intimate with the 'patch' format, as it lets users know 
> what git will do with your patch modifications.

Oh, that is not something I am against at all.  I (mis)took Peff's "what
makes sense" to cover more than what a patch format is, namely, "what the
user by modifying the patch and 'add -e' by accepting a modified patch are
doing."

> For example, when I first tried "add -e", my first thought was: "Awesome, 
> but, I wonder if git will do the right thing if I modify the patch in THIS 
> way ...". Fortunately, git did the right thing, but I wasn't really sure 
> until I tried it.

Actually, my "change +b to +e" example was meant to illustrate that git
is not necessarily doing the right thing.

Admittedly, there is no way to always do the right thing unless git can
read user's mind in this case, and realization of why it is impossible to
do is necessary before using "add -e"; otherwise you would end up getting
utterly confused.  I do not use "add -e" myself.

In one edit (i.e. "remove +c"), the intention is clear that the insertion
is merely kept out of the index and it might even eventually be added in a
later commit.  "c" could be a debugging "printf()" that the user may not
ever want to commit.  In either case, it is clear that the user wants to
keep that change in the work tree version.

In another edit (i.e. "change +b to +e"), the only thing that is clear is
that the user wants a version with "e" in the next commit.

Changing from "+b" to "+e" in "add -e" might have been done, in order to
change "+if (1 || debug)" in the work tree version to "+if (debug)" in the
version to be committed (so that the user can keep debugging without
giving the --debug option to the program, for example).  In this case, it
is quite similar to the earlier "remove +c" ("I know I inserted 'c' in my
work tree version, do not commit that change, but I do want to keep it in
my work tree") case, and keeping the work tree intact is the right thing
to do.

But it might have been done because the user spotted a typo in "b" and
wanted to correct it to "e", in which case the user may wish the change
be reflected to the work tree.

As there is no way to distinguish the last two cases (and I do not think
the code even attempts to tell the difference); half of the time you would
(perhaps mistakenly) think that "add -e" did a wrong thing to your typofix
edit, after adding the updated contents correctly to the index: "I told
git to fix the typo 'b' to 'e', and I committed the corrected version, but
then it discarded my fix---the typo is still there in my work tree".

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]