From: Erick Mattos Date: Tue, 03 Nov 2009 18:21:50 GMT Subject: Re: [PATCH] commit -c/-C/--amend: reset timestamp and authorship to committer with --mine 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 : > Erick Mattos writes: > >> 2009/11/1 Junio C Hamano : >>> Erick Mattos writes: >>> >>>>>    % git commit --claim --author='Erick 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. >>>>>> +     git commit -c HEAD <>>>>> +     "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.