threads / patch / 17967

patchadds --date=raw support to git blame and related documentation

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

## tl;dr

4 messages between Feb 23, 2009 and Feb 24, 2009. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

eletuchy@gmail.com· Feb 23, 2009, 08:57 UTC · lore
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(-)
Show changes to 5 files +10 −6

Documentation/blame-options.txt, Documentation/config.txt, Documentation/git-rev-list.txt, builtin-blame.c, contrib/completion/git-completion.bash

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
Junio C Hamano· Feb 23, 2009, 17:10 UTC · re: eletuchy@gmail.com · lore

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

eletuchy@gmail.com writes:
Show 5 quoted lines
> 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.

Show 10 quoted lines
> 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.

Show 12 quoted lines
> 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.

Show 7 quoted lines
>  	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.
Eugene Letuchy· Feb 23, 2009, 17:48 UTC · re: Junio C Hamano · lore

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

On 2/23/09 9:10 AM, Junio C Hamano wrote:
Show 18 quoted lines
> 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.
Show 18 quoted lines
>> 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.

Show 17 quoted lines
>> 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.

Show 10 quoted lines
>
>>   	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.
Junio C Hamano· Feb 24, 2009, 01:35 UTC · re: Eugene Letuchy · lore

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

Eugene Letuchy <eletuchy@facebook.com> writes:
Show 21 quoted lines
> 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.
Show 24 quoted lines
>>> 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?

← back to recent threads