[PATCH] Changed timestamp behavior of options -c/-C/--amend

[PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

The code herein changes commit timestamp recording from a source in a
more intuitive way.

Description:
When we use one of the options above we are normally trying to do mainly
two things: one is using the source as a template and second is to
recreate a commit with corrections.

In the first case timestamp should by default be taken by the time we
are doing the commit, not by the source.  On the second case the actual
behavior is acceptable.

Anyway this update creates new options for choosing the source timestamp
or a new one.  And set as default for -c option (editing one) to take a
new timestamp and for -C option the source timestamp.  That is because
we are normally using the source as template when we we are editing and
as a correction when we are just copying it.

Those options are also useful for --amend option which is by default
behaving the same.

Signed-off-by: Erick Mattos <erick.mattos@gmail.com>
---
 Documentation/git-commit.txt |   10 ++++++++--
 builtin-commit.c             |   15 ++++++++++++---
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 0578a40..27c61d2 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -10,6 +10,7 @@ SYNOPSIS
 [verse]
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
 	   [(-c | -C) <commit>] [-F <file> | -m <msg>]
+	   [(--old-timestamp | --new-timestamp)]
 	   [--allow-empty] [--no-verify] [-e] [--author=<author>]
 	   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
 
@@ -61,13 +62,16 @@ OPTIONS
 -C <commit>::
 --reuse-message=<commit>::
 	Take an existing commit object, and reuse the log message
-	and the authorship information (including the timestamp)
-	when creating the commit.
+	and the authorship information when creating the commit.
+	By default, timestamp is taken from specified commit unless
+	option --new-timestamp is included.
 
 -c <commit>::
 --reedit-message=<commit>::
 	Like '-C', but with '-c' the editor is invoked, so that
 	the user can further edit the commit message.
+	By default, timestamp is recalculated and not taken from
+	specified commit unless option --old-timestamp is included.
 
 -F <file>::
 --file=<file>::
@@ -134,6 +138,8 @@ OPTIONS
 	current tip -- if it was a merge, it will have the parents of
 	the current tip as parents -- so the current top commit is
 	discarded.
+	By default, timestamp is taken from latest commit unless option
+	--new-timestamp is included.
 +
 --
 It is a rough equivalent for:
diff --git a/builtin-commit.c b/builtin-commit.c
index c395cbf..a924584 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -52,6 +52,7 @@ static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run;
+static int old_timestamp, new_timestamp;
 static char *untracked_files_arg;
 /*
  * The default commit message cleanup mode will remove the lines
@@ -91,8 +92,10 @@ static struct option builtin_commit_options[] = {
 	OPT_FILENAME('F', "file", &logfile, "read log from file"),
 	OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
-	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
+	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+	OPT_BOOLEAN(0, "old-timestamp", &old_timestamp, "reuse previous commit's timestamp"),
+	OPT_BOOLEAN(0, "new-timestamp", &new_timestamp, "regenerate timestamp, ignoring previous one"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
 	OPT_BOOLEAN('e', "edit", &edit_flag, "force edit of commit"),
@@ -396,7 +399,8 @@ static void determine_author_info(void)
 
 		name = xstrndup(a + 8, lb - (a + 8));
 		email = xstrndup(lb + 2, rb - (lb + 2));
-		date = xstrndup(rb + 2, eol - (rb + 2));
+		if (!new_timestamp)
+			date = xstrndup(rb + 2, eol - (rb + 2));
 	}
 
 	if (force_author) {
@@ -763,11 +767,16 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		die("You have nothing to amend.");
 	if (amend && in_merge)
 		die("You are in the middle of a merge -- cannot amend.");
+	if (old_timestamp && new_timestamp)
+		die("You can not use --old-timesamp and --new-timestamp together.");
 
 	if (use_message)
 		f++;
-	if (edit_message)
+	if (edit_message) {
+		if (!old_timestamp)
+			new_timestamp = 1;
 		f++;
+	}
 	if (logfile)
 		f++;
 	if (f > 1)
-- 
1.6.5.2.102.g3ad0

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Jeff King]

On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:

> Anyway this update creates new options for choosing the source timestamp
> or a new one.  And set as default for -c option (editing one) to take a
> new timestamp and for -C option the source timestamp.  That is because
> we are normally using the source as template when we we are editing and
> as a correction when we are just copying it.
> 
> Those options are also useful for --amend option which is by default
> behaving the same.

Thanks, this is something I have been wanting. I have always thought
that --amend should give a new timestamp, so that while I'm fixing up
commits via "rebase -i" the commits end up in correct date order.

Your patch seems to always use the old timestamp for -C, the new one for
-c, and the old one for --amend. I would want it always for --amend.

I talked with Shawn about this at the GitTogether; his counter-argument
was that many people in maintainer roles will be amending or rebasing
just to do little things, like marking Signed-off-by, and that the date
should remain the same.

So my suspicion is that there are some people who almost always want
--new-timestamp, and some people who almost always want --old-timestamp,
depending on their usual workflow. In which case a config option
probably makes the most sense (but keeping the command-line to override
the config, of course).

> ---
>  Documentation/git-commit.txt |   10 ++++++++--
>  builtin-commit.c             |   15 ++++++++++++---
>  2 files changed, 20 insertions(+), 5 deletions(-)

Tests?

>  	Take an existing commit object, and reuse the log message
> -	and the authorship information (including the timestamp)
> -	when creating the commit.
> +	and the authorship information when creating the commit.
> +	By default, timestamp is taken from specified commit unless
> +	option --new-timestamp is included.

We should clarify that this is the _author_ timestamp. The committer
timestamp is always updated. Yes, it is talking about authorship
information in the previous sentence, but at least I had to read it
a few times to figure that out. Plus there are some minor English
tweaks needed, so maybe:

  and the authorship information when creating the commit.
  By default, the author timestamp is taken from the specified commit
  unless the option --new-timestamp is used.

>  -c <commit>::
>  --reedit-message=<commit>::
>  	Like '-C', but with '-c' the editor is invoked, so that
>  	the user can further edit the commit message.
> +	By default, timestamp is recalculated and not taken from
> +	specified commit unless option --old-timestamp is included.

Ditto:

  By default, the author timestamp is recalculated and not taken from
  the specified commit unless the option --old-timestamp is used.

> @@ -134,6 +138,8 @@ OPTIONS
>  	current tip -- if it was a merge, it will have the parents of
>  	the current tip as parents -- so the current top commit is
>  	discarded.
> +	By default, timestamp is taken from latest commit unless option
> +	--new-timestamp is included.

And:

  By default, the author timestamp is taken from the latest commit
  unless the option --new-timestamp is included.

> +	if (old_timestamp && new_timestamp)
> +		die("You can not use --old-timesamp and --new-timestamp together.");

Typo: s/samp/stamp/

But this mutual exclusivity implies to me that the option should
probably be "--timestamp=old|new".

-Peff

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

2009/10/30 Jeff King <peff@peff.net>:
> On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:
>
>> Anyway this update creates new options for choosing the source timestamp
>> or a new one.  And set as default for -c option (editing one) to take a
>> new timestamp and for -C option the source timestamp.  That is because
>> we are normally using the source as template when we we are editing and
>> as a correction when we are just copying it.
>>
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Thanks, this is something I have been wanting. I have always thought
> that --amend should give a new timestamp, so that while I'm fixing up
> commits via "rebase -i" the commits end up in correct date order.

You are welcome!


> Your patch seems to always use the old timestamp for -C, the new one for
> -c, and the old one for --amend. I would want it always for --amend.

About --amend: I didn't want to change the actual behavior and as a
matter of fact it means only adding the --new-timestamp option when
needed anyway.


> I talked with Shawn about this at the GitTogether; his counter-argument
> was that many people in maintainer roles will be amending or rebasing
> just to do little things, like marking Signed-off-by, and that the date
> should remain the same.
>
> So my suspicion is that there are some people who almost always want
> --new-timestamp, and some people who almost always want --old-timestamp,
> depending on their usual workflow. In which case a config option
> probably makes the most sense (but keeping the command-line to override
> the config, of course).

As you know you will find people defending both sides.  I am one that
prefers --amend the way it is now.  I really think that having an
option to change it is enough to keep peace.

I don't see a need for more changes in config because it would be imho
more complexity for a small need.

Of course you can do some alias in bash for setting mentioned
functionality up! ;-)


>> ---
>>  Documentation/git-commit.txt |   10 ++++++++--
>>  builtin-commit.c             |   15 ++++++++++++---
>>  2 files changed, 20 insertions(+), 5 deletions(-)
>
> Tests?

Yes indeed.  It is a change I am using which I thought it would be
useful for other people.  The code is quite simple too.  Simplicity
always lead to less possibility of bugs.  But yes, I had tested it.


>>       Take an existing commit object, and reuse the log message
>> -     and the authorship information (including the timestamp)
>> -     when creating the commit.
>> +     and the authorship information when creating the commit.
>> +     By default, timestamp is taken from specified commit unless
>> +     option --new-timestamp is included.
>
> We should clarify that this is the _author_ timestamp. The committer
> timestamp is always updated. Yes, it is talking about authorship
> information in the previous sentence, but at least I had to read it
> a few times to figure that out. Plus there are some minor English
> tweaks needed, so maybe:

I see your point but I do not think the way it is is confusing.  Of
course we will be talking about taste so I let it pass.  Anyway who
would think about or want to change the commiter timestamp?  Maybe a
fraudster!...  :-1


>  and the authorship information when creating the commit.
>  By default, the author timestamp is taken from the specified commit
>  unless the option --new-timestamp is used.
>
>>  -c <commit>::
>>  --reedit-message=<commit>::
>>       Like '-C', but with '-c' the editor is invoked, so that
>>       the user can further edit the commit message.
>> +     By default, timestamp is recalculated and not taken from
>> +     specified commit unless option --old-timestamp is included.
>
> Ditto:
>
>  By default, the author timestamp is recalculated and not taken from
>  the specified commit unless the option --old-timestamp is used.
>
>> @@ -134,6 +138,8 @@ OPTIONS
>>       current tip -- if it was a merge, it will have the parents of
>>       the current tip as parents -- so the current top commit is
>>       discarded.
>> +     By default, timestamp is taken from latest commit unless option
>> +     --new-timestamp is included.
>
> And:
>
>  By default, the author timestamp is taken from the latest commit
>  unless the option --new-timestamp is included.

As previously said I wouldn't change the text but anyway...


>> +     if (old_timestamp && new_timestamp)
>> +             die("You can not use --old-timesamp and --new-timestamp together.");
>
> Typo: s/samp/stamp/
>
> But this mutual exclusivity implies to me that the option should
> probably be "--timestamp=old|new".
>
> -Peff
>

Now it is a matter of taste again...  I prefer the options as I set up
because it is more the way other options are used so it is more
intuitive.  We use very little --'option'='something' using git.

Thanks for all your comments.  I have appreciated them very much.

The differences in opinion between us about the little details of this
update is not really relevant because we agree in the need of the
customization proposed.

I have really presented it because I thought it would be useful.

So I think the maintainer can accept it as he judges better.

One of the beauties of Free Software is: even if people don't agree on
a subject one can always compile and use the way one wants.  So
everybody gets satisfied!

Thank you very much again.

Best regards.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Oct 30, 2009 at 05:36:34PM -0200, Erick Mattos wrote:
>
>> Anyway this update creates new options for choosing the source timestamp
>> or a new one.  And set as default for -c option (editing one) to take a
>> new timestamp and for -C option the source timestamp.  That is because
>> we are normally using the source as template when we we are editing and
>> as a correction when we are just copying it.
>> 
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Thanks, this is something I have been wanting. I have always thought
> that --amend should give a new timestamp, so that while I'm fixing up
> commits via "rebase -i" the commits end up in correct date order.
>
> Your patch seems to always use the old timestamp for -C, the new one for
> -c, and the old one for --amend. I would want it always for --amend.
>
> I talked with Shawn about this at the GitTogether; his counter-argument
> was that many people in maintainer roles will be amending or rebasing
> just to do little things, like marking Signed-off-by, and that the date
> should remain the same.

Yeah, author timestamp shouldn't be molested for that kind of thing,
although we should update commit timestamp.

Yuck, was this about author timestamp?  Please then disregard my previous
response about the default.  I do not think there is strong reason to
change the default for any of them at all, even though giving people to
update what they committed with --no-reuse-timestamp would be a good
addition.

I also suspect that comparing committer and author name may give us a good
way to tweak the default in a more user friendly way.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Johannes Sixt]

Jeff King schrieb:
> But this mutual exclusivity implies to me that the option should
> probably be "--timestamp=old|new".

Even better:

	--timestamp=now
	--timestamp='2008-09-03 13:09:12 +0200'

I'd have needed this just yesterday...

-- Hannes

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Paolo Bonzini]

> So my suspicion is that there are some people who almost always want
> --new-timestamp, and some people who almost always want --old-timestamp,
> depending on their usual workflow. In which case a config option
> probably makes the most sense (but keeping the command-line to override
> the config, of course).

I'd say the config option should be per-branch (so that you can set the 
old-timestamp option only in integration branches, and not in topic 
branches), and that with such an option you could make the default be 
--new-timestamp in all three cases.

Paolo

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Erick Mattos <erick.mattos@gmail.com> writes:

A patch always changes something so the title "Changed ... behavior" does
not carry enough information (besides, you write logs as if you are making
an order to the codebase to "do this!").

> The code herein changes commit timestamp recording from a source in a
> more intuitive way.
>
> Description:

Remove the above.  Instead, start with a description of what the current
code does, e.g.

    Subject: commit -c/-C/--amend: allow 'current' timestamp to be used

    When these options are used, the timestamp recorded in the newly
    created commit is always taken from the original commit.

Then the rest of your text flows much more nicely...

> When we use one of the options above we are normally trying to do mainly
> two things: one is using the source as a template and second is to
> recreate a commit with corrections.
>
> In the first case timestamp should by default be taken by the time we
> are doing the commit, not by the source.  On the second case the actual
> behavior is acceptable.

... and the reader does not have to wonder what "the actual behavior" is;
instead you can say "the current behavior" here.

> ...
> Those options are also useful for --amend option which is by default
> behaving the same.

Also the reader does not have to wonder what "the same" means here.

I agree that the issue the patch addresses is worth improving, and I think
it is sensible to default to reuse the timestamp for -C and not to reuse
for --amend.  I am not sure about -c myself, but it probably shouldn't
reuse the timestamp by default.

I however think (old|new)-timestamp is suboptimal.

We already have --reuse-message, so why not trigger this with a single
option --(no-)reuse-timestamp?

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> ...
> I agree that the issue the patch addresses is worth improving, and I think
> it is sensible to default to reuse the timestamp for -C and not to reuse
> for --amend.  I am not sure about -c myself, but it probably shouldn't
> reuse the timestamp by default.

So after realizing that this was about "author" timestamp, I am rescinding
this comment about the change of the default for -c and --amend.

But everything else still stands.  IOW, I still (1) do think the issue is
worth addressing (thanks Erick), (2) the log message can be improved, and
(3) --(old|new)-timestamp should be --[no-]reuse-timestamp.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> ...
>> I agree that the issue the patch addresses is worth improving, and I think
>> it is sensible to default to reuse the timestamp for -C and not to reuse
>> for --amend.  I am not sure about -c myself, but it probably shouldn't
>> reuse the timestamp by default.
>
> So after realizing that this was about "author" timestamp, I am rescinding
> this comment about the change of the default for -c and --amend.

Actually I am only changing the default for -c and I see it useful.
At least with me I normally use -c only to use messages of commits as
template.

> But everything else still stands.  IOW, I still (1) do think the issue is
> worth addressing (thanks Erick), (2) the log message can be improved, and
> (3) --(old|new)-timestamp should be --[no-]reuse-timestamp.
>
(1) You are very welcome! :-)
(2) As you demand.
(3) I prefer the other way just because of saving some typing and
being more concise in my point of view.  But you know you decide it.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Erick Mattos <erick.mattos@gmail.com> writes:

> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> ...
>>> I agree that the issue the patch addresses is worth improving, and I think
>>> it is sensible to default to reuse the timestamp for -C and not to reuse
>>> for --amend.  I am not sure about -c myself, but it probably shouldn't
>>> reuse the timestamp by default.
>>
>> So after realizing that this was about "author" timestamp, I am rescinding
>> this comment about the change of the default for -c and --amend.
>
> Actually I am only changing the default for -c and I see it useful.
> At least with me I normally use -c only to use messages of commits as
> template.

I do that from time to time as well.  As I said in a different message, it
may make the default more intutitive if we give new timestamp when the
author is the same as the committer when doing "-c".  You are creating
your own commit in that case.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>>> Junio C Hamano <gitster@pobox.com> writes:
>>>
>>>> ...
>>>> I agree that the issue the patch addresses is worth improving, and I think
>>>> it is sensible to default to reuse the timestamp for -C and not to reuse
>>>> for --amend.  I am not sure about -c myself, but it probably shouldn't
>>>> reuse the timestamp by default.
>>>
>>> So after realizing that this was about "author" timestamp, I am rescinding
>>> this comment about the change of the default for -c and --amend.
>>
>> Actually I am only changing the default for -c and I see it useful.
>> At least with me I normally use -c only to use messages of commits as
>> template.
>
> I do that from time to time as well.  As I said in a different message, it
> may make the default more intutitive if we give new timestamp when the
> author is the same as the committer when doing "-c".  You are creating
> your own commit in that case.
>

I don't see a use for comparing the author and committer because I can
use as template my own commits or others'.

Let's clarify the subject:

In my point-of-view -c option is mainly used for templating commit messages.
In that case -c has a different default from -C and --amend options
thus creating a need for two new options: --reuse-timestamp and
--no-reuse-timestamp.

As I see by your messages you do prefer to have all those options set
up for reusing timestamp as default.
In that case we just need one new option: --no-reuse-timestamp (or
--recreate-timestamp or whatever).

So now It is a matter of decision only and you are the guy.

What should be for all?

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Erick Mattos <erick.mattos@gmail.com> writes:

> I don't see a use for comparing the author and committer because I can
> use as template my own commits or others'.

You _can_ use whichever irrelevant commit as a template, but "you _can_"
is different from what it means, and what is and what is not _sensible_.

You may be rewriting somebody else's patch (e.g. fixing up a typo in the
message, or changing the implementation, or both).  If you are going to
keep the authorship, you are saying that "it is still _his_ code, not
mine".  In such a case, it never makes sense to change the timestamp, if
that author is somebody other than you.  After all that other guy may not
even be aware of what you are doing when you make this commit; he may be
in bed sound asleep in a different timezone.

In another scenario, if your fix-up is very significant, even if you
started from somebody else's patch, you may want to say "now this is my
patch, the original author may have given me some inspiration, but the
changes in this commit, including all the bugs, are mine".  The same
applies if you looked at the problem description of somebody' patch, and
did your own solution without using anything from his commit.

At that point, you would want the resulting commit to say it was written
by you at this moment.  You do not want to see -c/-C/--amend to retain any
part of the authorship (not just timestamp) from the original commit.

    Side note. You may be fixing your own patch, in which case you may or
    may not consider your change significant, but at the time of either
    old timestamp or current time, you were working on this change, so
    using the current timestamp instead of using the old one is not a big
    deal, and that is why I think committer==author may be a good
    heuristic when deciding to touch or not touch the timestamp.

    But in general I do not like such dwim that depends on who you are (it
    makes it harder to explain, even if the end result may be useful in
    practice), so I'd rather not to see such a code for this topic if we
    can avoid it.

In short, I do not think it makes sense to change only the timestamp while
keeping the author.  The issue is not "timestamp behaviour" with "use new
timestamp" option, but rather is an ability to declare "Now this is a
commit made _by me_ and _now_; iow, I take authorship for this change",
even when you reuse the commit log message from somewhere else.

So what is needed is an option to tell -c/-C/--amend to reuse _only_ the
message but no authorship information from the original commit, I think.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

You are completely right.

All your concern is relevant and the whole problem must be re-engineered.

The good news is that I have almost finished it and I will be starting
a new thread with the new solution in a few minutes.

Regards

2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> I don't see a use for comparing the author and committer because I can
>> use as template my own commits or others'.
>
> You _can_ use whichever irrelevant commit as a template, but "you _can_"
> is different from what it means, and what is and what is not _sensible_.
>
> You may be rewriting somebody else's patch (e.g. fixing up a typo in the
> message, or changing the implementation, or both).  If you are going to
> keep the authorship, you are saying that "it is still _his_ code, not
> mine".  In such a case, it never makes sense to change the timestamp, if
> that author is somebody other than you.  After all that other guy may not
> even be aware of what you are doing when you make this commit; he may be
> in bed sound asleep in a different timezone.
>
> In another scenario, if your fix-up is very significant, even if you
> started from somebody else's patch, you may want to say "now this is my
> patch, the original author may have given me some inspiration, but the
> changes in this commit, including all the bugs, are mine".  The same
> applies if you looked at the problem description of somebody' patch, and
> did your own solution without using anything from his commit.
>
> At that point, you would want the resulting commit to say it was written
> by you at this moment.  You do not want to see -c/-C/--amend to retain any
> part of the authorship (not just timestamp) from the original commit.
>
>    Side note. You may be fixing your own patch, in which case you may or
>    may not consider your change significant, but at the time of either
>    old timestamp or current time, you were working on this change, so
>    using the current timestamp instead of using the old one is not a big
>    deal, and that is why I think committer==author may be a good
>    heuristic when deciding to touch or not touch the timestamp.
>
>    But in general I do not like such dwim that depends on who you are (it
>    makes it harder to explain, even if the end result may be useful in
>    practice), so I'd rather not to see such a code for this topic if we
>    can avoid it.
>
> In short, I do not think it makes sense to change only the timestamp while
> keeping the author.  The issue is not "timestamp behaviour" with "use new
> timestamp" option, but rather is an ability to declare "Now this is a
> commit made _by me_ and _now_; iow, I take authorship for this change",
> even when you reuse the commit log message from somewhere else.
>
> So what is needed is an option to tell -c/-C/--amend to reuse _only_ the
> message but no authorship information from the original commit, I think.
>

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
> A patch always changes something so the title "Changed ... behavior" does
> not carry enough information

Sorry but I thought It was enough.  First submitted patch.

>(besides, you write logs as if you are making
> an order to the codebase to "do this!").

Not a chance!  Just trying to help.  A way for paying back all the
benefits I enjoy by your software.

>> The code herein changes commit timestamp recording from a source in a
>> more intuitive way.
>>
>> Description:
>
> Remove the above.  Instead, start with a description of what the current
> code does, e.g.
>
>    Subject: commit -c/-C/--amend: allow 'current' timestamp to be used
>
>    When these options are used, the timestamp recorded in the newly
>    created commit is always taken from the original commit.

Demand accepted.

>
> Then the rest of your text flows much more nicely...
>
>> When we use one of the options above we are normally trying to do mainly
>> two things: one is using the source as a template and second is to
>> recreate a commit with corrections.
>>
>> In the first case timestamp should by default be taken by the time we
>> are doing the commit, not by the source.  On the second case the actual
>> behavior is acceptable.
>
> ... and the reader does not have to wonder what "the actual behavior" is;
> instead you can say "the current behavior" here.
>
>> ...
>> Those options are also useful for --amend option which is by default
>> behaving the same.
>
> Also the reader does not have to wonder what "the same" means here.

Done again.

> I agree that the issue the patch addresses is worth improving, and I think
> it is sensible to default to reuse the timestamp for -C and not to reuse
> for --amend.  I am not sure about -c myself, but it probably shouldn't
> reuse the timestamp by default.
>
> I however think (old|new)-timestamp is suboptimal.
>
> We already have --reuse-message, so why not trigger this with a single
> option --(no-)reuse-timestamp?
>
Don't you think it would be a little big?  I had compared the option
name so it would be more or less of reuse-message.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Junio C Hamano]

Erick Mattos <erick.mattos@gmail.com> writes:

> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>
>>(besides, you write logs as if you are making
>> an order to the codebase to "do this!").
>
> Not a chance!  Just trying to help.

Sorry, you misunderstood me.  What I meant was:

    It is customery for us to write our log messages as if the author of
    the patch is giving an order, iow, in imperative mood.  Your "Changed
    blah" violates that style.

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Erick Mattos]

2009/10/30 Junio C Hamano <gitster@pobox.com>:
> Erick Mattos <erick.mattos@gmail.com> writes:
>
>> 2009/10/30 Junio C Hamano <gitster@pobox.com>:
>>
>>>(besides, you write logs as if you are making
>>> an order to the codebase to "do this!").
>>
>> Not a chance!  Just trying to help.
>
> Sorry, you misunderstood me.  What I meant was:
>
>    It is customery for us to write our log messages as if the author of
>    the patch is giving an order, iow, in imperative mood.  Your "Changed
>    blah" violates that style.
>
I got it in the opposite way.  :-D

So one should give an "order"!...  :-1

Re: [PATCH] Changed timestamp behavior of options -c/-C/--amend

[Johannes Sixt]

Erick Mattos schrieb:
> @@ -61,13 +62,16 @@ OPTIONS
>  -C <commit>::
>  --reuse-message=<commit>::
>  	Take an existing commit object, and reuse the log message
> -	and the authorship information (including the timestamp)
> -	when creating the commit.
> +	and the authorship information when creating the commit.
> +	By default, timestamp is taken from specified commit unless
> +	option --new-timestamp is included.
>  
>  -c <commit>::
>  --reedit-message=<commit>::
>  	Like '-C', but with '-c' the editor is invoked, so that
>  	the user can further edit the commit message.
> +	By default, timestamp is recalculated and not taken from
> +	specified commit unless option --old-timestamp is included.
>  
>  -F <file>::
>  --file=<file>::
> @@ -134,6 +138,8 @@ OPTIONS
>  	current tip -- if it was a merge, it will have the parents of
>  	the current tip as parents -- so the current top commit is
>  	discarded.
> +	By default, timestamp is taken from latest commit unless option
> +	--new-timestamp is included.

I don't like this a lot. Currently, we consistently (and predictably!) 
reuse the timestamp when the author information is reused. Therefore, I 
think it should be sufficient to introduce only --new-timestamp.

-- Hannes