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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/23/09 9:10 AM, Junio C Hamano wrote:
eletuchy@xxxxxxxxx writes:

From: Eugene Letuchy<eugene@xxxxxxxxxxxx>

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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux