bug#32931: time-stamp-format: offer numeric time zones too

bug#32931: time-stamp-format: offer numeric time zones too

[積丹尼 Dan Jacobson]

time-stamp-format is a variable defined in ‘time-stamp.el’.

Alas must one must hardwire to get '+0800':
"%:y-%02m-%02d %02H:%02M:%02S +0800"

Because all are provided are:
%z   time zone name: ‘est’.		%Z  gives uppercase: ‘EST’

emacs-version "25.2.2"

bug#32931: time-stamp-format: offer numeric time zones too

[Lars Ingebrigtsen]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> time-stamp-format is a variable defined in ‘time-stamp.el’.
>
> Alas must one must hardwire to get '+0800':
> "%:y-%02m-%02d %02H:%02M:%02S +0800"
>
> Because all are provided are:
> %z   time zone name: ‘est’.		%Z  gives uppercase: ‘EST’

From reading the code, it looks like it's just using format-time-string,
so all the specs available there are available here.

I'll just remove all that stuff from the doc string and point users to
that function.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32931: time-stamp-format: offer numeric time zones too

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> From reading the code, it looks like it's just using format-time-string,
> so all the specs available there are available here.
>
> I'll just remove all that stuff from the doc string and point users to
> that function.

That was wrong; I didn't see this bit:

;;; FIXME This comment was written in 1996!
;;; time-stamp is transitioning to using the new, expanded capabilities
;;; of format-time-string.  During the process, this function implements
;;; intermediate, compatible formats and complains about old, soon to
;;; be unsupported, formats.  This function will get a lot (a LOT) shorter
;;; when the transition is complete and we can just pass most things
;;; straight through to format-time-string.
;;;      At all times, all the formats recommended in the doc string
;;; of time-stamp-format will work not only in the current version of
;;; Emacs, but in all versions that have been released within the past
;;; two years.
;;;      The : modifier is a temporary conversion feature used to resolve
;;; ambiguous formats--formats that are changing (over time) incompatibly.
(defun time-stamp-string-preprocess (format &optional time)

Sure takes a long time to transition.

But that function hard-codes all the stuff it accepts, and then passes
them piece-meal to format-time-string, so it's not as straightforward to
fix as I assumed...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32931: time-stamp-format: offer numeric time zones too

[Lars Ingebrigtsen]

Below is as far as I got in rewriting to use format-spec instead....
but there are many strange things in the time-stamp gallery of format
specs:

%:a  weekday name: `Monday'.		%#A gives uppercase: `MONDAY'

OK...

%3a  abbreviated weekday: `Mon'.	%3A gives uppercase: `MON'

Well, sure.

%:b  month name: `January'.		%#B gives uppercase: `JANUARY'
%3b  abbreviated month: `Jan'.		%3B gives uppercase: `JAN'

[...]

%#p  `am' or `pm'.			%P  gives uppercase: `AM' or `PM'

WTF!

Well, all of this can be translated into format-spec +
format-time-string things; I'll find the time some day, I guess...

diff --git a/lisp/time-stamp.el b/lisp/time-stamp.el
index f423683852..8157abbb3c 100644
--- a/lisp/time-stamp.el
+++ b/lisp/time-stamp.el
@@ -36,6 +36,8 @@
 
 ;;; Code:
 
+(require 'format-spec)
+
 (defgroup time-stamp nil
   "Maintain last change time stamps in files edited by Emacs."
   :group 'data
@@ -413,192 +415,33 @@ time-stamp-string
 (defconst time-stamp-no-file "(no file)"
   "String to use when the buffer is not associated with a file.")
 
-;;; FIXME This comment was written in 1996!
-;;; time-stamp is transitioning to using the new, expanded capabilities
-;;; of format-time-string.  During the process, this function implements
-;;; intermediate, compatible formats and complains about old, soon to
-;;; be unsupported, formats.  This function will get a lot (a LOT) shorter
-;;; when the transition is complete and we can just pass most things
-;;; straight through to format-time-string.
-;;;      At all times, all the formats recommended in the doc string
-;;; of time-stamp-format will work not only in the current version of
-;;; Emacs, but in all versions that have been released within the past
-;;; two years.
-;;;      The : modifier is a temporary conversion feature used to resolve
-;;; ambiguous formats--formats that are changing (over time) incompatibly.
 (defun time-stamp-string-preprocess (format &optional time)
   "Use a FORMAT to format date, time, file, and user information.
 Optional second argument TIME is only for testing.
 Implements non-time extensions to `format-time-string'
 and all `time-stamp-format' compatibility."
-  (let ((fmt-len (length format))
-	(ind 0)
-	cur-char
-	(prev-char nil)
-	(result "")
-	field-width
-	field-result
-	alt-form change-case
-	(paren-level 0))
-    (while (< ind fmt-len)
-      (setq cur-char (aref format ind))
-      (setq
-       result
-       (concat result
-      (cond
-       ((eq cur-char ?%)
-	;; eat any additional args to allow for future expansion
-	(setq alt-form nil change-case nil field-width "")
-	(while (progn
-		 (setq ind (1+ ind))
-		 (setq cur-char (if (< ind fmt-len)
-				    (aref format ind)
-				  ?\0))
-		 (or (eq ?. cur-char)
-		     (eq ?, cur-char) (eq ?: cur-char) (eq ?@ cur-char)
-		     (eq ?- cur-char) (eq ?+ cur-char) (eq ?_ cur-char)
-		     (eq ?\s cur-char) (eq ?# cur-char) (eq ?^ cur-char)
-		     (and (eq ?\( cur-char)
-			  (not (eq prev-char ?\\))
-			  (setq paren-level (1+ paren-level)))
-		     (if (and (eq ?\) cur-char)
-			      (not (eq prev-char ?\\))
-			      (> paren-level 0))
-			 (setq paren-level (1- paren-level))
-		       (and (> paren-level 0)
-			    (< ind fmt-len)))
-		     (if (and (<= ?0 cur-char) (>= ?9 cur-char))
-			 ;; get format width
-			 (let ((field-index ind))
-			   (while (progn
-				    (setq ind (1+ ind))
-				    (setq cur-char (if (< ind fmt-len)
-						       (aref format ind)
-						     ?\0))
-				    (and (<= ?0 cur-char) (>= ?9 cur-char))))
-			   (setq field-width (substring format field-index ind))
-			   (setq ind (1- ind))
-			   t))))
-	  (setq prev-char cur-char)
-	  ;; some characters we actually use
-	  (cond ((eq cur-char ?:)
-		 (setq alt-form t))
-		((eq cur-char ?#)
-		 (setq change-case t))))
-	(setq field-result
-	(cond
-	 ((eq cur-char ?%)
-	  "%%")
-	 ((eq cur-char ?a)		;day of week
-	  (if change-case
-	      (time-stamp--format "%#a" time)
-	    (or alt-form (not (string-equal field-width ""))
-		(time-stamp-conv-warn "%a" "%:a"))
-	    (if (and alt-form (not (string-equal field-width "")))
-		""			;discourage "%:3a"
-	      (time-stamp--format "%A" time))))
-	 ((eq cur-char ?A)
-	  (if alt-form
-	      (time-stamp--format "%A" time)
-	    (or change-case (not (string-equal field-width ""))
-		(time-stamp-conv-warn "%A" "%#A"))
-	    (time-stamp--format "%#A" time)))
-	 ((eq cur-char ?b)		;month name
-	  (if change-case
-	      (time-stamp--format "%#b" time)
-	    (or alt-form (not (string-equal field-width ""))
-		(time-stamp-conv-warn "%b" "%:b"))
-	    (if (and alt-form (not (string-equal field-width "")))
-		""			;discourage "%:3b"
-	    (time-stamp--format "%B" time))))
-	 ((eq cur-char ?B)
-	  (if alt-form
-	      (time-stamp--format "%B" time)
-	    (or change-case (not (string-equal field-width ""))
-		(time-stamp-conv-warn "%B" "%#B"))
-	    (time-stamp--format "%#B" time)))
-	 ((eq cur-char ?d)		;day of month, 1-31
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?H)		;hour, 0-23
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?I)		;hour, 1-12
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?m)		;month number, 1-12
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?M)		;minute, 0-59
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?p)		;am or pm
-	  (or change-case
-	      (time-stamp-conv-warn "%p" "%#p"))
-	  (time-stamp--format "%#p" time))
-	 ((eq cur-char ?P)		;AM or PM
-	  (time-stamp--format "%p" time))
-	 ((eq cur-char ?S)		;seconds, 00-60
-	  (time-stamp-do-number cur-char alt-form field-width time))
-	 ((eq cur-char ?w)		;weekday number, Sunday is 0
-	  (time-stamp--format "%w" time))
-	 ((eq cur-char ?y)		;year
-	  (or alt-form (not (string-equal field-width ""))
-	      (time-stamp-conv-warn "%y" "%:y"))
-	  (string-to-number (time-stamp--format "%Y" time)))
-	 ((eq cur-char ?Y)		;4-digit year, new style
-	  (string-to-number (time-stamp--format "%Y" time)))
-	 ((eq cur-char ?z)		;time zone lower case
-	  (if change-case
-	      ""			;discourage %z variations
-	    (time-stamp--format "%#Z" time)))
-	 ((eq cur-char ?Z)
-	  (if change-case
-	      (time-stamp--format "%#Z" time)
-	    (time-stamp--format "%Z" time)))
-	 ((eq cur-char ?f)		;buffer-file-name, base name only
-	  (if buffer-file-name
+  (format-spec
+   format
+   `((?f .                           ;buffer-file-name, base name only
+         ,(if buffer-file-name
 	      (file-name-nondirectory buffer-file-name)
 	    time-stamp-no-file))
-	 ((eq cur-char ?F)		;buffer-file-name, full path
-	  (or buffer-file-name
+     (?F .                              ;buffer-file-name, full path
+	 ,(or buffer-file-name
 	      time-stamp-no-file))
-	 ((eq cur-char ?s)		;system name
-	  (system-name))
-	 ((eq cur-char ?u)		;user name
-	  (user-login-name))
-	 ((eq cur-char ?U)		;user full name
-	  (user-full-name))
-	 ((eq cur-char ?l)		;logname (undocumented user name alt)
-	  (user-login-name))
-	 ((eq cur-char ?L)		;(undocumented alt user full name)
-	  (user-full-name))
-	 ((eq cur-char ?h)		;mail host name
-	  (or mail-host-address (system-name)))
-	 ((eq cur-char ?q)		;(undocumented unqual hostname)
-	  (let ((qualname (system-name)))
+     (?s . ,(system-name))       ; system name
+     (?u . ,(user-login-name))   ;user name
+     (?U . ,(user-full-name))    ;user full name
+     (?l . ,(user-login-name))   ;logname (undocumented user name alt)
+     (?L . ,(user-full-name))    ;(undocumented alt user full name)
+     (?h . ,(or mail-host-address (system-name))) ;mail host name
+     (?q .                             ;(undocumented unqual hostname)
+         ,(let ((qualname (system-name)))
 	    (if (string-match "\\." qualname)
 		(substring qualname 0 (match-beginning 0))
 	      qualname)))
-	 ((eq cur-char ?Q)		;(undocumented fully-qualified host)
-	  (system-name))
-	 ))
-	(let ((padded-result
-	       (format (format "%%%s%c"
-			       field-width
-			       (if (numberp field-result) ?d ?s))
-		       (or field-result ""))))
-	  (let* ((initial-length (length padded-result))
-		 (desired-length (if (string-equal field-width "")
-				     initial-length
-				   (string-to-number field-width))))
-	    (if (> initial-length desired-length)
-		;; truncate strings on right, years on left
-		(if (stringp field-result)
-		    (substring padded-result 0 desired-length)
-		  (if (eq cur-char ?y)
-		      (substring padded-result (- desired-length))
-		    padded-result))	;non-year numbers don't truncate
-	      padded-result))))
-       (t
-	(char-to-string cur-char)))))
-      (setq ind (1+ ind)))
-    result))
+     (?Q , (system-name)))        ;(undocumented fully-qualified host)
+   t))
 
 (defun time-stamp-do-number (format-char alt-form field-width time)
   "Handle compatible FORMAT-CHAR where only default width/padding will change.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#32931: time-stamp-format: offer numeric time zones too

[Stephen Gildea]

Version: 27.1

commit: 9ce67baa9a3733ae93885b6242130497b13c8703
time-stamp: implement numeric time zone conversion

* time-stamp.el: Implement %:z as expanding to the numeric time zone
offset, to address the feature request of bug#32931.  Do not document it
yet, to discourage compatibility problems in mixed Emacs 26 and Emacs 27
environments.  Documentation will be added in a subsequent release at
least two years later.  (We cannot yet use %z for numeric time zone
because in Emacs 26 it was documented to do something else.)

* time-stamp-tests.el (time-stamp-test-format-time-zone): expand this
test and break it into two tests, time-stamp-test-format-time-zone-name
and time-stamp-test-format-time-zone-offset.

bug#32931: time-stamp-format: offer numeric time zones too

[Robert Pluim]

>>>>> On Wed, 06 Nov 2019 08:48:46 -0800, Stephen Gildea <stepheng+emacs@gildea.com> said:

    Stephen> Version: 27.1
    Stephen> commit: 9ce67baa9a3733ae93885b6242130497b13c8703
    Stephen> time-stamp: implement numeric time zone conversion

    Stephen> * time-stamp.el: Implement %:z as expanding to the numeric time zone
    Stephen> offset, to address the feature request of bug#32931.  Do not document it
    Stephen> yet, to discourage compatibility problems in mixed Emacs 26 and Emacs 27
    Stephen> environments.  Documentation will be added in a subsequent release at
    Stephen> least two years later.  (We cannot yet use %z for numeric time zone
    Stephen> because in Emacs 26 it was documented to do something else.)

    Stephen> * time-stamp-tests.el (time-stamp-test-format-time-zone): expand this
    Stephen> test and break it into two tests, time-stamp-test-format-time-zone-name
    Stephen> and time-stamp-test-format-time-zone-offset.

If youʼre going to be making changes without documenting them, the
least you can do is follow the commit format conventions.

I donʼt see any reason for not documenting this, you can make it clear
that people mixing emacs 26 and 27 need to be careful.

Robert

bug#32931: time-stamp-format: offer numeric time zones too

[Eli Zaretskii]

> Date: Thu, 07 Nov 2019 10:02:06 +0100
> Cc: 32931@debbugs.gnu.org,
>  積丹尼 Dan Jacobson <jidanni@jidanni.org>
> 
> If youʼre going to be making changes without documenting them, the
> least you can do is follow the commit format conventions.
> 
> I donʼt see any reason for not documenting this, you can make it clear
> that people mixing emacs 26 and 27 need to be careful.

Indeed, please be sure to mention such changes in NEWS.  Bonus points
for updating the relevant manual(s) as well.

We had in the past important changes that were left undocumented
because we forgot to update NEWS and the manuals when the code was
added.  So I'd like to urge everybody to please go extra mile and push
documentation changes together with code changes.

TIA

bug#32931: time-stamp-format: offer numeric time zones too

[Stephen Gildea]

The time-stamp package has unusual compatibility issues that govern
when code changes are appropriate to document.  In the source, it is
noted that

;;;      At all times, all the formats recommended in the doc string
;;; of time-stamp-format will work not only in the current version of
;;; Emacs, but in all versions that have been released within the past
;;; two years.

The reason for this promise is that it is common for people to set
time-stamp-pattern or time-stamp-format in a local-variables section of
a file.  Files so annotated may be edited by different people and with
different versions of Emacs.  So that Emacs behaves consistently, the
time-stamp package recommends only formats that behave the same way
across all versions likely to be in simultaneous use.

In September I updated the documentation for changes implemented
previously; that commit included a section in NEWS.  I will eventually
document the change that sparked this discussion, too, and I'll add it
to NEWS at the same time.

So while I am aware of my responsibility to document, I am also
mindful of my responsibility to not document yet.

 < Stephen

bug#32931: time-stamp-format: offer numeric time zones too

[Eli Zaretskii]

> From: Stephen Gildea <stepheng+emacs@gildea.com>
> cc: 32931@debbugs.gnu.org, jidanni@jidanni.org,
>     Robert Pluim <rpluim@gmail.com>
> Date: Thu, 07 Nov 2019 12:57:49 -0800
> 
> In September I updated the documentation for changes implemented
> previously; that commit included a section in NEWS.  I will eventually
> document the change that sparked this discussion, too, and I'll add it
> to NEWS at the same time.
> 
> So while I am aware of my responsibility to document, I am also
> mindful of my responsibility to not document yet.

Thanks, but IME deferring the update of documentation for these
reasons is a mistake.  It frequently causes us to forget to make those
deferred updates, and more importantly, it leaves those who use the
development snapshots with outdated or lacking documentation.

I understand the wish to avoid unnecessary rework, but I don't think
there's a way around that.

bug#32931: time-stamp-format: offer numeric time zones too

[Stephen Gildea]

Eli, I would like to introduce this feature to time-stamp the same way
I have introduced other features: implement first, document later.
Implement in this release, document in a later release.

For example, the conversion "%q" (unqualified hostname) will be
new in Emacs 27.  It is newly described in the doc string of
time-stamp-format.  It is mentioned in 27.1 NEWS.  But the only
new thing is the documentation; a key point is that the feature
works in Emacs 26, too.  You can try it now:
(progn (require 'time-stamp) (time-stamp-string "%q"))

So why all this care with the timing of the time-stamp documentation?

Compatibility is difficult for time-stamp because people can set
time-stamp-pattern/time-stamp-format in their files.  This captures
the current protocol, and these user files have to be considered when
making any change to the time-stamp implementation or documentation.
When someone sets time-stamp-pattern/time-stamp-format as a local
variable, they are relying on a promise that the next time Emacs
(any Emacs) reads that file, the setting will be honored still.

Imagine that I don't do it this way, and a user sees a new feature.
(Perhaps the new feature is accompanied by a compatibility warning, but
the user thinks, "I use only my laptop, which I just upgraded to Emacs
27.1, so this is okay.")  They use the new feature in a local-variable
setting in their files.  Time passes, and they forget they have used
this new feature.  They give the file to a friend, or they edit it
from their work laptop, or they distribute the file on their git
repository.  By one of these paths, the file ends up getting edited
with a different Emacs, say version 24.5.  When that old Emacs saves
the file, it gets corrupted.

Given all the above, are you sure we want to document this now?

 < Stephen

bug#32931: time-stamp-format: offer numeric time zones too

[Eli Zaretskii]

> From: Stephen Gildea <stepheng+emacs@gildea.com>
> cc: 32931@debbugs.gnu.org, jidanni@jidanni.org, rpluim@gmail.com
> Date: Fri, 08 Nov 2019 16:09:37 -0800
> 
> Eli, I would like to introduce this feature to time-stamp the same way
> I have introduced other features: implement first, document later.
> Implement in this release, document in a later release.
> 
> For example, the conversion "%q" (unqualified hostname) will be
> new in Emacs 27.  It is newly described in the doc string of
> time-stamp-format.  It is mentioned in 27.1 NEWS.  But the only
> new thing is the documentation; a key point is that the feature
> works in Emacs 26, too.  You can try it now:
> (progn (require 'time-stamp) (time-stamp-string "%q"))

I think it's bad for us to have undocumented features.  We should have
documented %q in Emacs 26.  Why didn't we?

And what happens if between the implementation release and the
documentation release you change your interests and stop working on
this?  Who will remember that these features are implemented, but not
documented?

Finally, we have bug reports that complain about undocumented
features, or documentation that seemingly contradicts the code.  Are
you subscribed to bug-gnu-emacs and intend to take care of such bug
reports regarding the code you write in a timely fashion (read: before
I or someone else invests time and energy into investigating the
situation)?

IOW, I have my gray hair from code that was installed without
documentation to accompany it, for whatever reasons.  I'd therefore
like to minimize such situations, ideally to avoid them completely.

> So why all this care with the timing of the time-stamp documentation?

It isn't specific to time-stamp in any way.  It's a general concern
about our documentation being complete and up to date at any given
point in time.  We don't have dedicated people taking care of the
documentation, and our own efforts to add documentation post factum
proved in the past to be of insufficient quality.

> Compatibility is difficult for time-stamp because people can set
> time-stamp-pattern/time-stamp-format in their files.  This captures
> the current protocol, and these user files have to be considered when
> making any change to the time-stamp implementation or documentation.
> When someone sets time-stamp-pattern/time-stamp-format as a local
> variable, they are relying on a promise that the next time Emacs
> (any Emacs) reads that file, the setting will be honored still.
> 
> Imagine that I don't do it this way, and a user sees a new feature.
> (Perhaps the new feature is accompanied by a compatibility warning, but
> the user thinks, "I use only my laptop, which I just upgraded to Emacs
> 27.1, so this is okay.")  They use the new feature in a local-variable
> setting in their files.  Time passes, and they forget they have used
> this new feature.  They give the file to a friend, or they edit it
> from their work laptop, or they distribute the file on their git
> repository.  By one of these paths, the file ends up getting edited
> with a different Emacs, say version 24.5.  When that old Emacs saves
> the file, it gets corrupted.

I understand how changing the code can get users into these
situations, but I fail to understand how documentation can have
anything with them.  What did I miss?

> Given all the above, are you sure we want to document this now?

I think so, yes.  Unless I'm missing something important.

bug#32931: time-stamp-format: offer numeric time zones too

[Stephen Gildea]

>   > Given all the above, are you sure we want to document this now?
>   
>   I think so, yes.  Unless I'm missing something important.

Thank you for sharing your perspective, Eli.

I am working on a bug fix for the new format.  I'll include
documentation in that commit.

 < Stephen

bug#32931: time-stamp-format: offer numeric time zones too

[Eli Zaretskii]

> From: Stephen Gildea <stepheng+emacs@gildea.com>
> cc: 32931@debbugs.gnu.org, jidanni@jidanni.org, rpluim@gmail.com
> Date: Sat, 09 Nov 2019 08:51:19 -0800
> 
> I am working on a bug fix for the new format.  I'll include
> documentation in that commit.

Thanks in advance.

bug#32931: closed: bug#32931: time-stamp-format: offer numeric time zones too

[Stephen Gildea]

Version: 27.1

commit: 1d189843bb2a4f23dc269314d5e6dfb4f9fe801e
time-stamp: update support for time zone numeric offset

* time-stamp.el (time-stamp-string-preprocess): Change new format for
numeric time zone from %:z to %5z to match format-time-string better.
(time-stamp-format): Document support for numeric time zone.

* NEWS: Mention time-stamp-format %5z.