[PATCH] adds --date=raw support to git blame and related documentation

[PATCH] adds --date=raw support to git blame and related documentation

[eletuchy]

From: Eugene Letuchy <eugene@facebook.com>

In the wake of Linus' 7dff9b3, git blame --date support needs to
incorporate --date=raw in addition to the previously supported
date formats.

Test: > git grep relative | grep iso | grep -v raw
      > git blame --date=raw builtin-blame.c

Signed-off-by: Eugene Letuchy <eugene@facebook.com>
---
 Documentation/blame-options.txt        |    4 ++--
 Documentation/config.txt               |    2 +-
 Documentation/git-rev-list.txt         |    2 +-
 builtin-blame.c                        |    6 +++++-
 contrib/completion/git-completion.bash |    2 +-
 5 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index e6717af..1316d4e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -36,7 +36,7 @@ of lines before or after the line given by <start>.
 	Show long rev (Default: off).
 
 -t::
-	Show raw timestamp (Default: off).
+	Synomym for --date=raw (Default: off).
 
 -S <revs-file>::
 	Use revs from revs-file instead of calling linkgit:git-rev-list[1].
@@ -72,7 +72,7 @@ of lines before or after the line given by <start>.
 
 --date <format>::
 	The value is one of the following alternatives:
-	{relative,local,default,iso,rfc,short}. If --date is not
+	{relative,local,default,iso,rfc,short,raw}. If --date is not
 	provided, the value of the blame.date config variable is
 	used. If the blame.date config variable is also not set, the
 	iso format is used. For more information, See the discussion
diff --git a/Documentation/config.txt b/Documentation/config.txt
index f5152c5..f73d569 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1010,7 +1010,7 @@ interactive.singlekey::
 log.date::
 	Set default date-time mode for the log command. Setting log.date
 	value is similar to using 'git-log'\'s --date option. The value is one of the
-	following alternatives: {relative,local,default,iso,rfc,short}.
+	following alternatives: {relative,local,default,iso,rfc,short,raw}.
 	See linkgit:git-log[1].
 
 log.showroot::
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 1c9cc28..c226601 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -35,7 +35,7 @@ SYNOPSIS
 	     [ \--regexp-ignore-case | -i ]
 	     [ \--extended-regexp | -E ]
 	     [ \--fixed-strings | -F ]
-	     [ \--date={local|relative|default|iso|rfc|short} ]
+	     [ \--date={local|relative|default|iso|rfc|short|raw} ]
 	     [ [\--objects | \--objects-edge] [ \--unpacked ] ]
 	     [ \--pretty | \--header ]
 	     [ \--bisect ]
diff --git a/builtin-blame.c b/builtin-blame.c
index 48cedfd..bb0d20b 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2288,12 +2288,16 @@ parse_done:
 	case DATE_RELATIVE:
 		blame_date_width = sizeof("14 minutes ago");
 		break;
+	case DATE_RAW:
+		blame_date_width = sizeof("1235155266 -0800");
+		output_option |= OUTPUT_RAW_TIMESTAMP;
+		break;
 	case DATE_LOCAL:
 	case DATE_NORMAL:
 		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
 		break;
 	}
-	blame_date_width -= 1; /* strip the null */
+	blame_date_width -= 1; /* strip the terminating null */
 
 	if (DIFF_OPT_TST(&revs.diffopt, FIND_COPIES_HARDER))
 		opt |= (PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE |
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0a3092f..3454c1b 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1016,7 +1016,7 @@ _git_log ()
 		;;
 	--date=*)
 		__gitcomp "
-			relative iso8601 rfc2822 short local default
+			relative iso8601 rfc2822 short local default raw
 		" "" "${cur##--date=}"
 		return
 		;;
-- 
1.6.2.rc1.14.g07c3.dirty

Re: [PATCH] adds --date=raw support to git blame and related documentation

[Junio C Hamano]

eletuchy@gmail.com writes:

> From: Eugene Letuchy <eugene@facebook.com>
>
> In the wake of Linus' 7dff9b3, git blame --date support needs to
> incorporate --date=raw in addition to the previously supported
> date formats.

Thanks, but I do not understand what you meant by the following two lines:

> Test: > git grep relative | grep iso | grep -v raw
>       > git blame --date=raw builtin-blame.c

With the patch to add --date=raw format already on 'master', I'd prefer a
reroll of the original patch (it needs a fix for the config "don't ignore
a misconfiguration" bug Peff pointed out anyway) with this documentation
update patch squashed in.

> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> index e6717af..1316d4e 100644
> --- a/Documentation/blame-options.txt
> +++ b/Documentation/blame-options.txt
> @@ -36,7 +36,7 @@ of lines before or after the line given by <start>.
>  	Show long rev (Default: off).
>  
>  -t::
> -	Show raw timestamp (Default: off).
> +	Synomym for --date=raw (Default: off).

This is interesting.  It suggests that we should internally get rid of
show_raw_time variable (and need to error out when --date= and -t options
are given at the same time, as they are mutually incompatible).

But do -t and --date=raw really behave identically?  I think they should
but I didn't check.

> diff --git a/builtin-blame.c b/builtin-blame.c
> index 48cedfd..bb0d20b 100644
> --- a/builtin-blame.c
> +++ b/builtin-blame.c
> @@ -2288,12 +2288,16 @@ parse_done:
>  	case DATE_RELATIVE:
>  		blame_date_width = sizeof("14 minutes ago");
>  		break;
> +	case DATE_RAW:
> +		blame_date_width = sizeof("1235155266 -0800");
> +		output_option |= OUTPUT_RAW_TIMESTAMP;
> +		break;

I'd prefer it to see a same timestamp used consistently here.  You seem to
have used "Thu, 19 Oct 2006 16:00:04 -0700" for other case arms (I do not
know what significant things happened at that time) and what I queued in
'pu' has sizeof("1161298804 -0700") there instead.

>  	case DATE_LOCAL:
>  	case DATE_NORMAL:
>  		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
>  		break;
>  	}
> -	blame_date_width -= 1; /* strip the null */
> +	blame_date_width -= 1; /* strip the terminating null */

The character with byte value 0 is called NUL.

Thanks.

Re: [PATCH] adds --date=raw support to git blame and related documentation

[Eugene Letuchy]

On 2/23/09 9:10 AM, Junio C Hamano wrote:
> eletuchy@gmail.com writes:
>
>> From: Eugene Letuchy<eugene@facebook.com>
>>
>> In the wake of Linus' 7dff9b3, git blame --date support needs to
>> incorporate --date=raw in addition to the previously supported
>> date formats.
>
> Thanks, but I do not understand what you meant by the following two lines:
>
>> Test:>  git grep relative | grep iso | grep -v raw
>>        >  git blame --date=raw builtin-blame.c
>
> With the patch to add --date=raw format already on 'master', I'd prefer a
> reroll of the original patch (it needs a fix for the config "don't ignore
> a misconfiguration" bug Peff pointed out anyway) with this documentation
> update patch squashed in.
>

Yeah I can do that.

>> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
>> index e6717af..1316d4e 100644
>> --- a/Documentation/blame-options.txt
>> +++ b/Documentation/blame-options.txt
>> @@ -36,7 +36,7 @@ of lines before or after the line given by<start>.
>>   	Show long rev (Default: off).
>>
>>   -t::
>> -	Show raw timestamp (Default: off).
>> +	Synomym for --date=raw (Default: off).
>
> This is interesting.  It suggests that we should internally get rid of
> show_raw_time variable (and need to error out when --date= and -t options
> are given at the same time, as they are mutually incompatible).
>
> But do -t and --date=raw really behave identically?  I think they should
> but I didn't check.
>

The output of -t and --date=raw are exactly identical (well, after this patch 
they are); for that reason, I think providing both is redundant but not an 
error. However, I wanted to retain -t for "git annotate" compatibility, which 
has -t as the sole date option. In git-annotate mode, no other --date mode 
options can apply.

>> diff --git a/builtin-blame.c b/builtin-blame.c
>> index 48cedfd..bb0d20b 100644
>> --- a/builtin-blame.c
>> +++ b/builtin-blame.c
>> @@ -2288,12 +2288,16 @@ parse_done:
>>   	case DATE_RELATIVE:
>>   		blame_date_width = sizeof("14 minutes ago");
>>   		break;
>> +	case DATE_RAW:
>> +		blame_date_width = sizeof("1235155266 -0800");
>> +		output_option |= OUTPUT_RAW_TIMESTAMP;
>> +		break;
>
> I'd prefer it to see a same timestamp used consistently here.  You seem to
> have used "Thu, 19 Oct 2006 16:00:04 -0700" for other case arms (I do not
> know what significant things happened at that time) and what I queued in
> 'pu' has sizeof("1161298804 -0700") there instead.

Thu, 19 Oct 2006 16:00:04 -0700 is the date for the first line of builtin-blame.c
1161298804 -0700 is fine by me.

>
>>   	case DATE_LOCAL:
>>   	case DATE_NORMAL:
>>   		blame_date_width = sizeof("Thu Oct 19 16:00:04 2006 -0700");
>>   		break;
>>   	}
>> -	blame_date_width -= 1; /* strip the null */
>> +	blame_date_width -= 1; /* strip the terminating null */
>
> The character with byte value 0 is called NUL.

OK.

>
> Thanks.

Re: [PATCH] adds --date=raw support to git blame and related documentation

[Junio C Hamano]

Eugene Letuchy <eletuchy@facebook.com> writes:

> On 2/23/09 9:10 AM, Junio C Hamano wrote:
>> eletuchy@gmail.com writes:
>>
>>> From: Eugene Letuchy<eugene@facebook.com>
>>>
>>> In the wake of Linus' 7dff9b3, git blame --date support needs to
>>> incorporate --date=raw in addition to the previously supported
>>> date formats.
>>
>> Thanks, but I do not understand what you meant by the following two lines:
>>
>>> Test:>  git grep relative | grep iso | grep -v raw
>>>        >  git blame --date=raw builtin-blame.c
>>
>> With the patch to add --date=raw format already on 'master', I'd prefer a
>> reroll of the original patch (it needs a fix for the config "don't ignore
>> a misconfiguration" bug Peff pointed out anyway) with this documentation
>> update patch squashed in.
>>
>
> Yeah I can do that.

Thanks.

>>> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
>>> index e6717af..1316d4e 100644
>>> --- a/Documentation/blame-options.txt
>>> +++ b/Documentation/blame-options.txt
>>> @@ -36,7 +36,7 @@ of lines before or after the line given by<start>.
>>>   	Show long rev (Default: off).
>>>
>>>   -t::
>>> -	Show raw timestamp (Default: off).
>>> +	Synomym for --date=raw (Default: off).
>>
>> This is interesting.  It suggests that we should internally get rid of
>> show_raw_time variable (and need to error out when --date= and -t options
>> are given at the same time, as they are mutually incompatible).
>>
>> But do -t and --date=raw really behave identically?  I think they should
>> but I didn't check.
>>
>
> The output of -t and --date=raw are exactly identical (well, after
> this patch they are); for that reason, I think providing both is
> redundant but not an error. However, I wanted to retain -t for "git
> annotate" compatibility, which has -t as the sole date option. In
> git-annotate mode, no other --date mode options can apply.

Oh, I didn't mean combination of -t and --date=raw.  Consider a
combination such as -t and --date=iso given together.

The removal of show_raw_time would be a good idea if -t and --date=raw
are meant to be identical, right?