Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --mine

From: Erick Mattos <erick.mattos@gmail.com>
Date: Nov 3, 2009, 18:21 UTC
Message-ID: <55bacdd30911031021s6d5ac03aka0f0b5aae273db1@mail.gmail.com>

Please don't forget your comments down here wasn't about the last sent patch. Please see it, in case you don't have it, at: http://marc.info/?l=git&m=125712272606721&w=2

Anyway I am answering your comments down here:

2009/11/2 Junio C Hamano <gitster@pobox.com>:

Show 28 quoted lines

> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> 2009/11/1 Junio C Hamano <gitster@pobox.com>:
>>> Erick Mattos <erick.mattos@gmail.com> writes:
>>>
>>>>>    % git commit --claim --author='Erick Mattos <eric@mattos>' -C HEAD
>>>>>
>>>>> Should you detect an error?  Does your code do so?  Do you have a test
>>>>> that catches this error?
>>>>
>>>> It works as intended.  Both together.
>>>
>>> That does not make any sense.  If you are saying this is yours and it is
>>> his at the same time, there can be no sane way to work "as intended", no?.
>>
>> I am adding a new option not changing the option --author already in
>> git.  So it does work together.
>
> Somebody who says "this commit is mine, and its author is this other
> person" is not making any sense.  The resulting commit can either have
> that person (i.e. the committer) as the author, which is what the "claim"
> option means, or it can have the person named with --author as the author,
> but both cannot be true at the same time.
>
> When you introduce a new option, sometimes it cannot sanely be used with
> an existing option.  In such a case, two options (the new one and the
> existing one) are called mutually exclusive.  And you add some code to
> catch an user error to use them together.

As --author text says: "override author for commit". As I see, something that OVERRIDES supersedes everything else. IMHO --author shouldn't be blocked by the new option. Probably your point is about "mine" name. Cutting --author away would make impossible for someone to force a new author with a new timestamp in case he is templating. Of course he can be using the --author because he is doing a change in a computer not his own or something alike. So I would not wipe "author" out from the new option. Please don't forget that I am just being a small contributor. I am just suggesting things. You have the final word.

Show 17 quoted lines

>>>>>> +     git commit -c HEAD <<EOF
>>>>>> +     "Changed"
>>>>>> +     EOF &&
>>>>>
>>>>> What editor is reading this "Changed"?
>>>>
>>>> Nobody cares...  Just a text to change the file.
>>>
>>> I actually care.  Who uses that Changed string, and where does it end up
>>> with?  At the end of the log message?  At the beginning?  What "file"?
>>
>> I didn't get it.  -c option does not accept -m option and starts an
>> editor to change the message.  The text "Changed is just a forced
>> message.  I can not use an editor in interactive mode in a script...
>
> How are the existing tests that try "commit -c" do this?  I do not think
> there is any here-text redirect into "git commit".

Sorry, it was automatic for me. Just supposing a here-text... :-) I am going to fix it.

> It is sometimes easier to show by example than by giving nudging words
> that only show direction, so here is a suggested rewrite on top of your
> patch.  I am not very happy with the option name "mine" either, but at
> least I think this gets the semantics right.

We could call it --reset-author.  What do you think?

> +       if (force_author && renew_authorship)
> +               die("Using both --mine and --author does not make sense");
> +

As previously said up there.