git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 0/6] reflog docs and date-formatting
@ 2016-07-22 19:51 Jeff King
  2016-07-22 19:51 ` [PATCH v2 1/6] doc/rev-list-options: clarify "commit@{Nth}" for "-g" option Jeff King
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

This is a repost of the patches in:

  http://thread.gmane.org/gmane.comp.version-control.git/299201/focus=299236

since I don't think they got picked up at all.

The contents are the same, but with one extra patch (the 4th) that was
posted mid-discussion.

The only other review comment is that patch 5 might want to editorialize
the weirdness of "raw-local" more. I think I'm comfortable leaving it
as-is based on my response in:

  http://article.gmane.org/gmane.comp.version-control.git/299312

but I could be persuaded otherwise.

The final patch, "--date=unix" doesn't _really_ solve Ted's problem,
because there's still no way to give multiple date formats in the same
line, nor use "%gd" as both the reflog index and as the date. So I'm
assuming he'll still build "%gt" or similar on top of this, and only in
the long run will we get to "%(reflog-date:unix)" or whatever.

So I think it's still valuable as a minor feature for other formatting
needs, and as a potential building block for later. The earlier patches
are all immediately useful as clarifications.

  [1/6]: doc/rev-list-options: clarify "commit@{Nth}" for "-g" option
  [2/6]: doc/rev-list-options: explain "-g" output formats
  [3/6]: doc/pretty-formats: describe index/time formats for %gd
  [4/6]: doc/pretty-formats: explain shortening of %gd
  [5/6]: date: document and test "raw-local" mode
  [6/6]: date: add "unix" format

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] doc/rev-list-options: clarify "commit@{Nth}" for "-g" option
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
@ 2016-07-22 19:51 ` Jeff King
  2016-07-22 19:51 ` [PATCH v2 2/6] doc/rev-list-options: explain "-g" output formats Jeff King
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

When "log -g" shows "HEAD@{1}", "HEAD@{2}", etc, calling
that "commit@{Nth}" is not really accurate. The "HEAD" part
is really the refname. By saying "commit", a reader may
misunderstand that to mean something related to the specific
commit we are showing, not the ref whose reflog we are
traversing.

While we're here, let's also switch these instances to use
literal backticks, as our style guide recommends. As a
bonus, that lets us drop some asciidoc quoting.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/rev-list-options.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index c5bd218..1b56253 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -252,9 +252,9 @@ list.
 +
 With `--pretty` format other than `oneline` (for obvious reasons),
 this causes the output to have two extra lines of information
-taken from the reflog.  By default, 'commit@\{Nth}' notation is
+taken from the reflog.  By default, `ref@{Nth}` notation is
 used in the output.  When the starting commit is specified as
-'commit@\{now}', output also uses 'commit@\{timestamp}' notation
+`ref@{now}`, output also uses `ref@{timestamp}` notation
 instead.  Under `--pretty=oneline`, the commit message is
 prefixed with this information on the same line.
 This option cannot be combined with `--reverse`.
-- 
2.9.2.512.gc1ef750


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] doc/rev-list-options: explain "-g" output formats
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
  2016-07-22 19:51 ` [PATCH v2 1/6] doc/rev-list-options: clarify "commit@{Nth}" for "-g" option Jeff King
@ 2016-07-22 19:51 ` Jeff King
  2016-07-22 19:51 ` [PATCH v2 3/6] doc/pretty-formats: describe index/time formats for %gd Jeff King
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

We document that asking for HEAD@{now} will switch the
output to show HEAD@{timestamp}, but not that specifying
`--date` has a similar effect, or that it can be overridden
with HEAD@{0}. Let's do so.

These rules come from 794151e (reflog-walk: always make
HEAD@{0} show indexed selectors, 2012-05-04), though that is
simply the culmination of years of these heuristics growing
organically.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/rev-list-options.txt | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1b56253..5d1de06 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -252,10 +252,25 @@ list.
 +
 With `--pretty` format other than `oneline` (for obvious reasons),
 this causes the output to have two extra lines of information
-taken from the reflog.  By default, `ref@{Nth}` notation is
-used in the output.  When the starting commit is specified as
-`ref@{now}`, output also uses `ref@{timestamp}` notation
-instead.  Under `--pretty=oneline`, the commit message is
+taken from the reflog.  The reflog designator in the output may be shown
+as `ref@{Nth}` (where `Nth` is the reverse-chronological index in the
+reflog) or as `ref@{timestamp}` (with the timestamp for that entry),
+depending on a few rules:
++
+--
+1. If the starting point is specified as `ref@{Nth}`, show the index
+format.
++
+2. If the starting point was specified as `ref@{now}`, show the
+timestamp format.
++
+3. If neither was used, but `--date` was given on the command line, show
+the timestamp in the format requested by `--date`.
++
+4. Otherwise, show the index format.
+--
++
+Under `--pretty=oneline`, the commit message is
 prefixed with this information on the same line.
 This option cannot be combined with `--reverse`.
 See also linkgit:git-reflog[1].
-- 
2.9.2.512.gc1ef750


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] doc/pretty-formats: describe index/time formats for %gd
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
  2016-07-22 19:51 ` [PATCH v2 1/6] doc/rev-list-options: clarify "commit@{Nth}" for "-g" option Jeff King
  2016-07-22 19:51 ` [PATCH v2 2/6] doc/rev-list-options: explain "-g" output formats Jeff King
@ 2016-07-22 19:51 ` Jeff King
  2016-07-22 19:51 ` [PATCH v2 4/6] doc/pretty-formats: explain shortening of %gd Jeff King
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

The "reflog selector" format changes based on a series of
heuristics, and that applies equally to both stock "log -g"
output, as well as "--format=%gd". The documentation for
"%gd" doesn't cover this. Let's mention the multiple formats
and refer the user back to the "-g" section for the complete
rules.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/pretty-formats.txt | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 29b19b9..36a300a 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -147,8 +147,11 @@ endif::git-rev-list[]
   "U" for a good signature with unknown validity and "N" for no signature
 - '%GS': show the name of the signer for a signed commit
 - '%GK': show the key used to sign a signed commit
-- '%gD': reflog selector, e.g., `refs/stash@{1}`
-- '%gd': shortened reflog selector, e.g., `stash@{1}`
+- '%gD': reflog selector, e.g., `refs/stash@{1}` or
+  `refs/stash@{2 minutes ago`}; the format follows the rules described
+  for the `-g` option
+- '%gd': shortened reflog selector, e.g., `stash@{1}` or
+  `stash@{2 minutes ago}`
 - '%gn': reflog identity name
 - '%gN': reflog identity name (respecting .mailmap, see
   linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- 
2.9.2.512.gc1ef750


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] doc/pretty-formats: explain shortening of %gd
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
                   ` (2 preceding siblings ...)
  2016-07-22 19:51 ` [PATCH v2 3/6] doc/pretty-formats: describe index/time formats for %gd Jeff King
@ 2016-07-22 19:51 ` Jeff King
  2016-07-22 19:51 ` [PATCH v2 5/6] date: document and test "raw-local" mode Jeff King
  2016-07-22 19:51 ` [PATCH v2 6/6] date: add "unix" format Jeff King
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

The actual shortening rules aren't that interesting and
probably not worth getting into (I gloss over them here as
"shortened for human readability"). But the fact that %gD
shows whatever you gave on the command line is subtle and
worth mentioning. Since most people will feed a shortened
refname in the first place, it otherwise makes it hard to
understand the difference between the two.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/pretty-formats.txt | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Documentation/pretty-formats.txt b/Documentation/pretty-formats.txt
index 36a300a..b95d67e 100644
--- a/Documentation/pretty-formats.txt
+++ b/Documentation/pretty-formats.txt
@@ -149,9 +149,12 @@ endif::git-rev-list[]
 - '%GK': show the key used to sign a signed commit
 - '%gD': reflog selector, e.g., `refs/stash@{1}` or
   `refs/stash@{2 minutes ago`}; the format follows the rules described
-  for the `-g` option
-- '%gd': shortened reflog selector, e.g., `stash@{1}` or
-  `stash@{2 minutes ago}`
+  for the `-g` option. The portion before the `@` is the refname as
+  given on the command line (so `git log -g refs/heads/master` would
+  yield `refs/heads/master@{0}`).
+- '%gd': shortened reflog selector; same as `%gD`, but the refname
+  portion is shortened for human readability (so `refs/heads/master`
+  becomes just `master`).
 - '%gn': reflog identity name
 - '%gN': reflog identity name (respecting .mailmap, see
   linkgit:git-shortlog[1] or linkgit:git-blame[1])
-- 
2.9.2.512.gc1ef750


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] date: document and test "raw-local" mode
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
                   ` (3 preceding siblings ...)
  2016-07-22 19:51 ` [PATCH v2 4/6] doc/pretty-formats: explain shortening of %gd Jeff King
@ 2016-07-22 19:51 ` Jeff King
  2016-07-23 10:15   ` Jakub Narębski
  2016-07-22 19:51 ` [PATCH v2 6/6] date: add "unix" format Jeff King
  5 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

The "raw" format shows a Unix epoch timestamp, but with a
timezone tacked on. The timestamp is not _in_ that zone, but
it is extra information about the time (by default, the zone
the author was in).

The documentation claims that "raw-local" does not work. It
does, but the end result is rather subtle. Let's describe it
in better detail, and test to make sure it works (namely,
the epoch time doesn't change, but the zone does).

While we are rewording the documentation in this area, let's
not use the phrase "does not work" for the remaining option,
"--relative". It's vague; do we accept it or not? We do
accept it, but it has no effect (which is a reasonable
outcome).

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/rev-list-options.txt | 9 ++++++---
 t/t0006-date.sh                    | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5d1de06..3ec75d4 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -725,8 +725,8 @@ include::pretty-options.txt[]
 	`iso-local`), the user's local time zone is used instead.
 +
 `--date=relative` shows dates relative to the current time,
-e.g. ``2 hours ago''. The `-local` option cannot be used with
-`--raw` or `--relative`.
+e.g. ``2 hours ago''. The `-local` option has no effect for
+`--relative`.
 +
 `--date=local` is an alias for `--date=default-local`.
 +
@@ -746,7 +746,10 @@ format, often found in email messages.
 +
 `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
 +
-`--date=raw` shows the date in the internal raw Git format `%s %z` format.
+`--date=raw` shows the date in the internal raw Git format `%s %z`
+format. Note that the `-local` option does not affect the
+seconds-since-epoch value (which is always measured in UTC), but does
+switch the accompanying timezone value.
 +
 `--date=format:...` feeds the format `...` to your system `strftime`.
 Use `--date=format:%c` to show the date in your system locale's
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 4c8cf58..482fec0 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -47,6 +47,7 @@ check_show short "$TIME" '2016-06-15'
 check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '1466000000 +0200'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
+check_show raw-local "$TIME" '1466000000 +0000'
 
 # arbitrary time absurdly far in the future
 FUTURE="5758122296 -0400"
-- 
2.9.2.512.gc1ef750


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] date: add "unix" format
  2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
                   ` (4 preceding siblings ...)
  2016-07-22 19:51 ` [PATCH v2 5/6] date: document and test "raw-local" mode Jeff King
@ 2016-07-22 19:51 ` Jeff King
  5 siblings, 0 replies; 16+ messages in thread
From: Jeff King @ 2016-07-22 19:51 UTC (permalink / raw)
  To: git; +Cc: Theodore Ts'o, Junio C Hamano

We already have "--date=raw", which is a Unix epoch
timestamp plus a contextual timezone (either the author's or
the local). But one may not care about the timezone and just
want the epoch timestamp by itself. It's not hard to parse
the two apart, but if you are using a pretty-print format,
you may want git to show the "finished" form that the user
will see.

We can accomodate this by adding a new date format, "unix",
which is basically "raw" without the timezone.

Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/rev-list-options.txt | 4 ++++
 builtin/blame.c                    | 3 +++
 cache.h                            | 3 ++-
 date.c                             | 8 ++++++++
 t/t0006-date.sh                    | 2 ++
 5 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 3ec75d4..8b1c946 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -751,6 +751,10 @@ format. Note that the `-local` option does not affect the
 seconds-since-epoch value (which is always measured in UTC), but does
 switch the accompanying timezone value.
 +
+`--date=unix` shows the date as a Unix epoch timestamp (seconds since
+1970).  As with `--raw`, this is always in UTC and therefore `-local`
+has no effect.
++
 `--date=format:...` feeds the format `...` to your system `strftime`.
 Use `--date=format:%c` to show the date in your system locale's
 preferred format.  See the `strftime` manual for a complete list of
diff --git a/builtin/blame.c b/builtin/blame.c
index 8fec0e1..cb08127 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2625,6 +2625,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 	case DATE_RAW:
 		blame_date_width = sizeof("1161298804 -0700");
 		break;
+	case DATE_UNIX:
+		blame_date_width = sizeof("1161298804");
+		break;
 	case DATE_SHORT:
 		blame_date_width = sizeof("2006-10-19");
 		break;
diff --git a/cache.h b/cache.h
index 2bf97cc..dd587b2 100644
--- a/cache.h
+++ b/cache.h
@@ -1224,7 +1224,8 @@ struct date_mode {
 		DATE_ISO8601_STRICT,
 		DATE_RFC2822,
 		DATE_STRFTIME,
-		DATE_RAW
+		DATE_RAW,
+		DATE_UNIX
 	} type;
 	const char *strftime_fmt;
 	int local;
diff --git a/date.c b/date.c
index 4c7aa9b..a996331 100644
--- a/date.c
+++ b/date.c
@@ -177,6 +177,12 @@ const char *show_date(unsigned long time, int tz, const struct date_mode *mode)
 	struct tm *tm;
 	static struct strbuf timebuf = STRBUF_INIT;
 
+	if (mode->type == DATE_UNIX) {
+		strbuf_reset(&timebuf);
+		strbuf_addf(&timebuf, "%lu", time);
+		return timebuf.buf;
+	}
+
 	if (mode->local)
 		tz = local_tzoffset(time);
 
@@ -792,6 +798,8 @@ static enum date_mode_type parse_date_type(const char *format, const char **end)
 		return DATE_NORMAL;
 	if (skip_prefix(format, "raw", end))
 		return DATE_RAW;
+	if (skip_prefix(format, "unix", end))
+		return DATE_UNIX;
 	if (skip_prefix(format, "format", end))
 		return DATE_STRFTIME;
 
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 482fec0..c0c9108 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -46,8 +46,10 @@ check_show rfc2822 "$TIME" 'Wed, 15 Jun 2016 16:13:20 +0200'
 check_show short "$TIME" '2016-06-15'
 check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '1466000000 +0200'
+check_show unix "$TIME" '1466000000'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
 check_show raw-local "$TIME" '1466000000 +0000'
+check_show unix-local "$TIME" '1466000000'
 
 # arbitrary time absurdly far in the future
 FUTURE="5758122296 -0400"
-- 
2.9.2.512.gc1ef750

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/6] date: document and test "raw-local" mode
  2016-07-22 19:51 ` [PATCH v2 5/6] date: document and test "raw-local" mode Jeff King
@ 2016-07-23 10:15   ` Jakub Narębski
  2016-07-26 18:47     ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Narębski @ 2016-07-23 10:15 UTC (permalink / raw)
  To: Jeff King, git; +Cc: Theodore Ts'o, Junio C Hamano

W dniu 2016-07-22 o 21:51, Jeff King pisze:

> The "raw" format shows a Unix epoch timestamp, but with a
> timezone tacked on. The timestamp is not _in_ that zone, but
> it is extra information about the time (by default, the zone
> the author was in).

I would say that "Unix time" (name according to Wikipedia) is
zone-less, but the information is correct.

> 
> The documentation claims that "raw-local" does not work. It
> does, but the end result is rather subtle. Let's describe it
> in better detail, and test to make sure it works (namely,
> the epoch time doesn't change, but the zone does).

Subtle, and not very useful (unless you want to know your
own timezone difference)... or do we adjust timezone offset
shown across DST change for the locale?

> 
> While we are rewording the documentation in this area, let's
> not use the phrase "does not work" for the remaining option,
> "--relative". It's vague; do we accept it or not? We do
> accept it, but it has no effect (which is a reasonable
> outcome).

Anyway, replacing vague "does not work" with explanation of
what is the result is a very good idea.

> 
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  Documentation/rev-list-options.txt | 9 ++++++---
>  t/t0006-date.sh                    | 1 +
>  2 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 5d1de06..3ec75d4 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -725,8 +725,8 @@ include::pretty-options.txt[]
>  	`iso-local`), the user's local time zone is used instead.
>  +
>  `--date=relative` shows dates relative to the current time,
> -e.g. ``2 hours ago''. The `-local` option cannot be used with
> -`--raw` or `--relative`.
> +e.g. ``2 hours ago''. The `-local` option has no effect for
> +`--relative`.

Do I understand it correctly: --relative is a short form for more
generic --date=relative (which probably should be spelled 
--date-format=relative), and that --date=relative-local is the
same as --date=relative, that is *-local suffix does not change
how date is formatted?

Because I don't think you can say --relative-local ("The `-local`
option has no effect on `--relative`"), can you?

Nevertheless the proposed change is definite improvement.

>  +
>  `--date=local` is an alias for `--date=default-local`.
>  +
> @@ -746,7 +746,10 @@ format, often found in email messages.
>  +
>  `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
>  +
> -`--date=raw` shows the date in the internal raw Git format `%s %z` format.
> +`--date=raw` shows the date in the internal raw Git format `%s %z`
> +format. Note that the `-local` option does not affect the
> +seconds-since-epoch value (which is always measured in UTC), but does
> +switch the accompanying timezone value.

Which is correct, logical, and next to useless, I think.


BTW. one kind of format that Git does not support (I think) is the
varying kind, where the precision changes with the distance from now,
so that we can get most precision in limited width.  That's what
`ls --long` does:

 * 'Jun 29 16:47' for dates falling in current year (more precision)
 * 'Nov 23  2015' for dates outside (less precision, same width)

Many other programs switch from relative to absolute time when date
in question is far in the past that relative is not very good.

>  +
>  `--date=format:...` feeds the format `...` to your system `strftime`.
>  Use `--date=format:%c` to show the date in your system locale's
> diff --git a/t/t0006-date.sh b/t/t0006-date.sh
> index 4c8cf58..482fec0 100755
> --- a/t/t0006-date.sh
> +++ b/t/t0006-date.sh
> @@ -47,6 +47,7 @@ check_show short "$TIME" '2016-06-15'
>  check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
>  check_show raw "$TIME" '1466000000 +0200'
>  check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
> +check_show raw-local "$TIME" '1466000000 +0000'
>  
>  # arbitrary time absurdly far in the future
>  FUTURE="5758122296 -0400"
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/6] date: document and test "raw-local" mode
  2016-07-23 10:15   ` Jakub Narębski
@ 2016-07-26 18:47     ` Jeff King
  2016-07-27 12:35       ` Jakub Narębski
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-07-26 18:47 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: git, Theodore Ts'o, Junio C Hamano

On Sat, Jul 23, 2016 at 12:15:37PM +0200, Jakub Narębski wrote:

> > diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> > index 5d1de06..3ec75d4 100644
> > --- a/Documentation/rev-list-options.txt
> > +++ b/Documentation/rev-list-options.txt
> > @@ -725,8 +725,8 @@ include::pretty-options.txt[]
> >  	`iso-local`), the user's local time zone is used instead.
> >  +
> >  `--date=relative` shows dates relative to the current time,
> > -e.g. ``2 hours ago''. The `-local` option cannot be used with
> > -`--raw` or `--relative`.
> > +e.g. ``2 hours ago''. The `-local` option has no effect for
> > +`--relative`.
> 
> Do I understand it correctly: --relative is a short form for more
> generic --date=relative (which probably should be spelled 
> --date-format=relative), and that --date=relative-local is the
> same as --date=relative, that is *-local suffix does not change
> how date is formatted?
> 
> Because I don't think you can say --relative-local ("The `-local`
> option has no effect on `--relative`"), can you?

All correct. There is no --relative-local because "--relative" is a
historical artifact. We could support --foo for every --date=foo, but I
don't think there is a reason to do so (and reasons not to, like
avoiding cluttering the option space).

> > -`--date=raw` shows the date in the internal raw Git format `%s %z` format.
> > +`--date=raw` shows the date in the internal raw Git format `%s %z`
> > +format. Note that the `-local` option does not affect the
> > +seconds-since-epoch value (which is always measured in UTC), but does
> > +switch the accompanying timezone value.
> 
> Which is correct, logical, and next to useless, I think.

This was discussed in the earlier review. It's basically only useful if
you are feeding the output to another script which will format the date
and want to change what that script shows.

> BTW. one kind of format that Git does not support (I think) is the
> varying kind, where the precision changes with the distance from now,
> so that we can get most precision in limited width.  That's what
> `ls --long` does:
> 
>  * 'Jun 29 16:47' for dates falling in current year (more precision)
>  * 'Nov 23  2015' for dates outside (less precision, same width)
> 
> Many other programs switch from relative to absolute time when date
> in question is far in the past that relative is not very good.

Yes, this was discussed on the list not too long ago. I think it would
be useful, but is obviously orthogonal to this series.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/6] date: document and test "raw-local" mode
  2016-07-26 18:47     ` Jeff King
@ 2016-07-27 12:35       ` Jakub Narębski
  2016-07-27 13:44         ` [PATCH v3 " Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Jakub Narębski @ 2016-07-27 12:35 UTC (permalink / raw)
  To: Jeff King; +Cc: git, Theodore Ts'o, Junio C Hamano

W dniu 2016-07-26 o 20:47, Jeff King pisze:
> On Sat, Jul 23, 2016 at 12:15:37PM +0200, Jakub Narębski wrote:
> 
>>> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
>>> index 5d1de06..3ec75d4 100644
>>> --- a/Documentation/rev-list-options.txt
>>> +++ b/Documentation/rev-list-options.txt
>>> @@ -725,8 +725,8 @@ include::pretty-options.txt[]
>>>  	`iso-local`), the user's local time zone is used instead.
>>>  +
>>>  `--date=relative` shows dates relative to the current time,
>>> -e.g. ``2 hours ago''. The `-local` option cannot be used with
>>> -`--raw` or `--relative`.
>>> +e.g. ``2 hours ago''. The `-local` option has no effect for
>>> +`--relative`.
>>
>> Do I understand it correctly: --relative is a short form for more
>> generic --date=relative (which probably should be spelled 
>> --date-format=relative), and that --date=relative-local is the
>> same as --date=relative, that is *-local suffix does not change
>> how date is formatted?
>>
>> Because I don't think you can say --relative-local ("The `-local`
>> option has no effect on `--relative`"), can you?
> 
> All correct. There is no --relative-local because "--relative" is a
> historical artifact. We could support --foo for every --date=foo, but I
> don't think there is a reason to do so (and reasons not to, like
> avoiding cluttering the option space).

So shouldn't the last sentence 

  "The `-local` option has no effect for `--relative`." 

be rather

 "The `-local` option has no effect for `--date=relative`."

Though this might be just me being overly nitpicky...

-- 
Jakub Narębski


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 12:35       ` Jakub Narębski
@ 2016-07-27 13:44         ` Jeff King
  2016-07-27 19:49           ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-07-27 13:44 UTC (permalink / raw)
  To: Jakub Narębski; +Cc: git, Theodore Ts'o, Junio C Hamano

On Wed, Jul 27, 2016 at 02:35:08PM +0200, Jakub Narębski wrote:

> > All correct. There is no --relative-local because "--relative" is a
> > historical artifact. We could support --foo for every --date=foo, but I
> > don't think there is a reason to do so (and reasons not to, like
> > avoiding cluttering the option space).
> 
> So shouldn't the last sentence 
> 
>   "The `-local` option has no effect for `--relative`." 
> 
> be rather
> 
>  "The `-local` option has no effect for `--date=relative`."

Ah, I see. I didn't realize that's what you are getting at. Yes, it
would be more accurate. That's not a new problem introduced by my patch;
I just happened to be touching that sentence for other reasons. But the
patch does try to clean up the text in general, so it seems like a good
time to make this fix, too.

Since it looks like this is not even in 'next' yet, here is a
replacement for the 5th patch (second from the top) of the
jk/reflog-date topic.

-- >8 --
Subject: [PATCH] date: document and test "raw-local" mode

The "raw" format shows a Unix epoch timestamp, but with a
timezone tacked on. The timestamp is not _in_ that zone, but
it is extra information about the time (by default, the zone
the author was in).

The documentation claims that "raw-local" does not work. It
does, but the end result is rather subtle. Let's describe it
in better detail, and test to make sure it works (namely,
the epoch time doesn't change, but the zone does).

While we are rewording the documentation in this area, let's
not use the phrase "does not work" for the remaining option,
"--date=relative". It's vague; do we accept it or not? We do
accept it, but it has no effect (which is a reasonable
outcome). We should also refer to the option not as
"--relative" (which is the historical synonym, and does not
take "-local" at all), but as "--date=relative".

Helped-by: Jakub Narębski <jnareb@gmail.com>
Signed-off-by: Jeff King <peff@peff.net>
---
 Documentation/rev-list-options.txt | 9 ++++++---
 t/t0006-date.sh                    | 1 +
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5d1de06..04c7c64 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -725,8 +725,8 @@ include::pretty-options.txt[]
 	`iso-local`), the user's local time zone is used instead.
 +
 `--date=relative` shows dates relative to the current time,
-e.g. ``2 hours ago''. The `-local` option cannot be used with
-`--raw` or `--relative`.
+e.g. ``2 hours ago''. The `-local` option has no effect for
+`--date=relative`.
 +
 `--date=local` is an alias for `--date=default-local`.
 +
@@ -746,7 +746,10 @@ format, often found in email messages.
 +
 `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
 +
-`--date=raw` shows the date in the internal raw Git format `%s %z` format.
+`--date=raw` shows the date in the internal raw Git format `%s %z`
+format. Note that the `-local` option does not affect the
+seconds-since-epoch value (which is always measured in UTC), but does
+switch the accompanying timezone value.
 +
 `--date=format:...` feeds the format `...` to your system `strftime`.
 Use `--date=format:%c` to show the date in your system locale's
diff --git a/t/t0006-date.sh b/t/t0006-date.sh
index 4c8cf58..482fec0 100755
--- a/t/t0006-date.sh
+++ b/t/t0006-date.sh
@@ -47,6 +47,7 @@ check_show short "$TIME" '2016-06-15'
 check_show default "$TIME" 'Wed Jun 15 16:13:20 2016 +0200'
 check_show raw "$TIME" '1466000000 +0200'
 check_show iso-local "$TIME" '2016-06-15 14:13:20 +0000'
+check_show raw-local "$TIME" '1466000000 +0000'
 
 # arbitrary time absurdly far in the future
 FUTURE="5758122296 -0400"
-- 
2.9.2.607.g98dce7b


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 13:44         ` [PATCH v3 " Jeff King
@ 2016-07-27 19:49           ` Junio C Hamano
  2016-07-27 19:57             ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2016-07-27 19:49 UTC (permalink / raw)
  To: Jeff King; +Cc: Jakub Narębski, git, Theodore Ts'o

Jeff King <peff@peff.net> writes:

> +e.g. ``2 hours ago''. The `-local` option has no effect for
> +`--date=relative`.

Good.

> @@ -746,7 +746,10 @@ format, often found in email messages.
>  +
>  `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
>  +
> -`--date=raw` shows the date in the internal raw Git format `%s %z` format.
> +`--date=raw` shows the date in the internal raw Git format `%s %z`
> +format. Note that the `-local` option does not affect the
> +seconds-since-epoch value (which is always measured in UTC), but does
> +switch the accompanying timezone value.

Not your fault, but "internal raw Git format X format" sounds a bit
clunky.

	shows the date in `%s %z` format (seconds since 1970-01-01
	00:00:00 UTC, space, timezone as sign and four digits)
        
or something like that?  I suspect %s is not a POSIX-kosher strftime
conversion specifier, so from that point of view, perhaps making the
text the authoritative description and demote the format string as a
comment to help those who know strfmt, i.e.

	shows the date as seconds since 1970-01-01 00:00:00 UTC,
	space, timezone as sign and four digits (i.e. as if the
	timestamp were formatted with `strftime("%s %z")`).

perhaps?


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 19:49           ` Junio C Hamano
@ 2016-07-27 19:57             ` Jeff King
  2016-07-27 20:11               ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-07-27 19:57 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narębski, git, Theodore Ts'o

On Wed, Jul 27, 2016 at 12:49:40PM -0700, Junio C Hamano wrote:

> > @@ -746,7 +746,10 @@ format, often found in email messages.
> >  +
> >  `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
> >  +
> > -`--date=raw` shows the date in the internal raw Git format `%s %z` format.
> > +`--date=raw` shows the date in the internal raw Git format `%s %z`
> > +format. Note that the `-local` option does not affect the
> > +seconds-since-epoch value (which is always measured in UTC), but does
> > +switch the accompanying timezone value.
> 
> Not your fault, but "internal raw Git format X format" sounds a bit
> clunky.

Yeah, I also noticed that while writing the original, but it just
crossed over my line of "if I fix everything I don't like, this series
will never get out the door". :)

> 	shows the date in `%s %z` format (seconds since 1970-01-01
> 	00:00:00 UTC, space, timezone as sign and four digits)
>         
> or something like that?  I suspect %s is not a POSIX-kosher strftime
> conversion specifier, so from that point of view, perhaps making the
> text the authoritative description and demote the format string as a
> comment to help those who know strfmt, i.e.
> 
> 	shows the date as seconds since 1970-01-01 00:00:00 UTC,
> 	space, timezone as sign and four digits (i.e. as if the
> 	timestamp were formatted with `strftime("%s %z")`).

Yeah, I think referring to "%s %z" is pretty bad, as it's meaningless to
people who don't know strftime() off the top of their head, and %s isn't
even portable.

The second option you've shown here is much better, and I think this is
worth fixing (and it should be in a new patch, and can go on top of the
others).

I'm not sure if it's worth calling out more clearly what the timezone
means, since the epoch seconds are in UTC, as you noted here. The
follow-on sentence explains what "-local" does, but not what it means in
the non-local variant.

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 19:57             ` Jeff King
@ 2016-07-27 20:11               ` Junio C Hamano
  2016-07-27 20:16                 ` Jeff King
  0 siblings, 1 reply; 16+ messages in thread
From: Junio C Hamano @ 2016-07-27 20:11 UTC (permalink / raw)
  To: Jeff King; +Cc: Jakub Narębski, git, Theodore Ts'o

Jeff King <peff@peff.net> writes:

> The second option you've shown here is much better, and I think this is
> worth fixing (and it should be in a new patch, and can go on top of the
> others).
>
> I'm not sure if it's worth calling out more clearly what the timezone
> means, since the epoch seconds are in UTC, as you noted here. The
> follow-on sentence explains what "-local" does, but not what it means in
> the non-local variant.

Hmm, that is another thing we can incrementally improve, I guess.

Here is 7/6 I've tentatively queued.  Note that I minimized the
reflowing of the paragraph you did in 5/6 so the patch may not apply
cleanly to your tree.

-- >8 --
Subject: [PATCH] date: clarify --date=raw description

"... in the internal raw Git format `%s %z` format." was clunky in
repeating "format" twice, and would not have helped those who do not
know that these are strftime(3) conversion specifiers.

Explain them with words, and demote the mention of `%s %z` to a
hint to help those who know them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/rev-list-options.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index fd86ed1..d55e3ae 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -746,7 +746,9 @@ format, often found in email messages.
 +
 `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
 +
-`--date=raw` shows the date in the internal raw Git format `%s %z` format.
+`--date=raw` shows the date as seconds since 1970-01-01 00:00:00 UTC,
+space, timezone as sign and four digits (i.e. as if the timestamp were
+formatted with `strftime("%s %z")`).
 Note that the `-local` option does not affect the seconds-since-epoch
 value (which is always measured in UTC), but does switch the accompanying
 timezone value.
-- 
2.9.2-654-g4c312ab


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 20:11               ` Junio C Hamano
@ 2016-07-27 20:16                 ` Jeff King
  2016-07-27 20:17                   ` Junio C Hamano
  0 siblings, 1 reply; 16+ messages in thread
From: Jeff King @ 2016-07-27 20:16 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jakub Narębski, git, Theodore Ts'o

On Wed, Jul 27, 2016 at 01:11:34PM -0700, Junio C Hamano wrote:

> -- >8 --
> Subject: [PATCH] date: clarify --date=raw description
> 
> "... in the internal raw Git format `%s %z` format." was clunky in
> repeating "format" twice, and would not have helped those who do not
> know that these are strftime(3) conversion specifiers.
> 
> Explain them with words, and demote the mention of `%s %z` to a
> hint to help those who know them.

Good explanation.

> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index fd86ed1..d55e3ae 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -746,7 +746,9 @@ format, often found in email messages.
>  +
>  `--date=short` shows only the date, but not the time, in `YYYY-MM-DD` format.
>  +
> -`--date=raw` shows the date in the internal raw Git format `%s %z` format.
> +`--date=raw` shows the date as seconds since 1970-01-01 00:00:00 UTC,
> +space, timezone as sign and four digits (i.e. as if the timestamp were
> +formatted with `strftime("%s %z")`).

I do find this by itself a little hard to parse. Maybe:

  `--date=raw` shows the date as seconds since the epoch (1970-01-01
  00:00:00 UTC), followed by a space, and then the timezone as an offset
  from UTC (a `+` or `-` with four digits; the first two are hours, and
  the second two are minutes). I.e., as if the timestamp were formatted
  with `strftime("%s %z")`).

-Peff

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 5/6] date: document and test "raw-local" mode
  2016-07-27 20:16                 ` Jeff King
@ 2016-07-27 20:17                   ` Junio C Hamano
  0 siblings, 0 replies; 16+ messages in thread
From: Junio C Hamano @ 2016-07-27 20:17 UTC (permalink / raw)
  To: Jeff King; +Cc: Jakub Narębski, git, Theodore Ts'o

Jeff King <peff@peff.net> writes:

> I do find this by itself a little hard to parse. Maybe:
>
>   `--date=raw` shows the date as seconds since the epoch (1970-01-01
>   00:00:00 UTC), followed by a space, and then the timezone as an offset
>   from UTC (a `+` or `-` with four digits; the first two are hours, and
>   the second two are minutes). I.e., as if the timestamp were formatted
>   with `strftime("%s %z")`).

I also had trouble with what I wrote.  Thanks for a great help.


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-07-27 20:17 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-22 19:51 [PATCH v2 0/6] reflog docs and date-formatting Jeff King
2016-07-22 19:51 ` [PATCH v2 1/6] doc/rev-list-options: clarify "commit@{Nth}" for "-g" option Jeff King
2016-07-22 19:51 ` [PATCH v2 2/6] doc/rev-list-options: explain "-g" output formats Jeff King
2016-07-22 19:51 ` [PATCH v2 3/6] doc/pretty-formats: describe index/time formats for %gd Jeff King
2016-07-22 19:51 ` [PATCH v2 4/6] doc/pretty-formats: explain shortening of %gd Jeff King
2016-07-22 19:51 ` [PATCH v2 5/6] date: document and test "raw-local" mode Jeff King
2016-07-23 10:15   ` Jakub Narębski
2016-07-26 18:47     ` Jeff King
2016-07-27 12:35       ` Jakub Narębski
2016-07-27 13:44         ` [PATCH v3 " Jeff King
2016-07-27 19:49           ` Junio C Hamano
2016-07-27 19:57             ` Jeff King
2016-07-27 20:11               ` Junio C Hamano
2016-07-27 20:16                 ` Jeff King
2016-07-27 20:17                   ` Junio C Hamano
2016-07-22 19:51 ` [PATCH v2 6/6] date: add "unix" format Jeff King

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).