Refactor `org-time-stamp-custom-formats' and `org-time-stamp-formats'

* lisp/org.el (org-time-stamp-formats):
* lisp/org.el (org-time-stamp-custom-formats): Change the default
values stripping leading "<" and trailing ">".  Update the docstring
explaining the format and that leading and trailing brackets are now
ignored.  Update the :type specification to more precise.
(org-time-stamp-format): Update the argument list and docstring
allowing to use the function more flexibly to find the time stamp
format for both `org-time-stamp-formats' and
`org-time-stamp-custom-formats'.  Rename `long' argument to more
accurate `with-time'.  Ignore brackets in the `org-time-stamp-formats'
and `org-time-stamp-custom-formats'.  Allow `inactive' argument to be
`no-brackets'
(org-format-timestamp):
(org-read-date-display):
(org-insert-time-stamp):
(org-display-custom-time):
(org-timestamp-translate):
* lisp/org-compat.el (org-timestamp-format): Rename
`org-timestamp-format' to `org-format-timestamp'.  The old variant is
too similar with other `org-time-stamp-format' function.  Also, use
`org-time-stamp-format' to determine the timestamp format instead of
using `org-time-stamp-formats' directly.
* lisp/ol.el (org-store-link):
* lisp/org-agenda.el (org-agenda-get-timestamps):
(org-agenda-get-progress):
* lisp/org-archive.el (org-archive-subtree):
(org-archive-to-archive-sibling):
* lisp/org-clock.el (org-clock-special-range):
* lisp/org-colview.el (org-colview-construct-allowed-dates):
* lisp/org-element.el (org-element-timestamp-interpreter):
* lisp/org-macro.el (org-macro--find-date):
* lisp/org-pcomplete.el (pcomplete/org-mode/file-option/date):
* lisp/ox-odt.el (org-odt--format-timestamp):
(org-odt-template):
* lisp/ox.el (org-export-get-date):
* testing/lisp/test-org.el (test-org/timestamp-format): Use
`org-time-stamp-format' instead of directly examining
`org-time-stamp-custom-formats' and `org-time-stamp-formats'.  Use the
new function name `org-format-timestamp'.
* etc/ORG-NEWS (Default values and interpretations of ~org-time-stamp-formats~ and ~org-time-stamp-custom-formats~ are changed):
(~org-timestamp-format~ is renamed to ~org-format-timestamp~):
(Updated argument list in ~org-time-stamp-format~): Document the
user-facing changes.

This commit documents and unifies previously undocumented assumptions
about the values of `org-time-stamp-formats' and
`org-time-stamp-custom-formats'.  Instead of fiddling with
leading/trailing brackets in the values, expedite the time format
calculation to `org-time-stamp-format'.  The undocumented assumption
about brackets in user option `org-time-stamp-custom-formats' is not
relaxed making the docstring correct.

Reported-by: Uwe Brauer <oub@mat.ucm.es>
Link: https://orgmode.org/list/87k04ppp1t.fsf@localhost

etc/ORG-NEWS

@@ -460,6 +460,36 @@ exported and an ASCII graph will be inserted below the src block.
 
 The new variable name is =org-plantuml-args=.  It now applies to both
 jar PlantUML file and executable.
+*** Default values and interpretations of ~org-time-stamp-formats~ and ~org-time-stamp-custom-formats~ are changed
+
+Leading =<= and trailing =>= in the default values of
+~org-time-stamp-formats~ and ~org-time-stamp-custom-formats~ are
+stripped.
+
+The Org functions that are using these variables also ignore leading
+and trailing brackets (=<...>= and =[...]=, if present).
+
+This change makes the Org code more consistent and also makes the
+docstring for ~org-time-stamp-custom-formats~ accurate.
+
+No changes on the user side are needed if
+~org-time-stamp-custom-formats~ was customized.
+*** ~org-timestamp-format~ is renamed to ~org-format-timestamp~
+
+The old function name is similar to other ~org-time-stamp-format~
+function.  The new name emphasizes that ~org-format-timestamp~ works
+on =timestamp= objects.
+
+*** Updated argument list in ~org-time-stamp-format~
+
+New =custom= argument in ~org-time-stamp-format~ makes the function
+use ~org-time-stamp-custom-formats~ instead of
+~org-time-stamp-formats~ to determine the format.
+
+Optional argument =long= is renamed to =with-time=, emphasizing that it refers to time stamp format with time specification.
+
+Optional argument =inactive= can now have a value =no-brackets= to
+return format string with brackets stripped.
 
 ** Miscellaneous
 *** SQL Babel ~:dbconnection~ parameter can be mixed with other SQL Babel parameters

lisp/ol.el

@@ -42,7 +42,6 @@
 (defvar org-inhibit-startup)
 (defvar org-outline-regexp-bol)
 (defvar org-src-source-file-name)
-(defvar org-time-stamp-formats)
 (defvar org-ts-regexp)
 
 (declare-function calendar-cursor-to-date "calendar" (&optional error event))
@@ -1620,7 +1619,7 @@ non-nil."
 	(let ((cd (calendar-cursor-to-date)))
 	  (setq link
 		(format-time-string
-		 (car org-time-stamp-formats)
+                 (org-time-stamp-format)
 		 (org-encode-time 0 0 0 (nth 1 cd) (nth 0 cd) (nth 2 cd))))
 	  (org-link-store-props :type "calendar" :date cd)))
 

lisp/org-agenda.el

@@ -5809,7 +5809,7 @@ displayed in agenda view."
 	   (regexp-quote
 	    (substring
 	     (format-time-string
-	      (car org-time-stamp-formats)
+	      (org-time-stamp-format)
 	      (org-encode-time	; DATE bound by calendar
 	       0 0 0 (nth 1 date) (car date) (nth 2 date)))
 	     1 11))
@@ -6092,7 +6092,7 @@ then those holidays will be skipped."
 		  (regexp-quote
 		   (substring
 		    (format-time-string
-		     (car org-time-stamp-formats)
+		     (org-time-stamp-format)
 		     (org-encode-time  ; DATE bound by calendar
 		      0 0 0 (nth 1 date) (car date) (nth 2 date)))
 		    1 11))))

lisp/org-archive.el

@@ -239,7 +239,7 @@ direct children of this heading."
 	     (tr-org-odd-levels-only org-odd-levels-only)
 	     (this-buffer (current-buffer))
 	     (time (format-time-string
-		    (substring (cdr org-time-stamp-formats) 1 -1)))
+                    (org-time-stamp-format 'with-time 'no-brackets)))
 	     (file (abbreviate-file-name
 		    (or (buffer-file-name (buffer-base-buffer))
 			(error "No file associated to buffer"))))
@@ -490,7 +490,7 @@ Archiving time is retained in the ARCHIVE_TIME node property."
 	(org-set-property
 	 "ARCHIVE_TIME"
 	 (format-time-string
-	  (substring (cdr org-time-stamp-formats) 1 -1)))
+          (org-time-stamp-format 'with-time 'no-brackets)))
 	(outline-up-heading 1 t)
 	(org-fold-subtree t)
 	(org-cycle-show-empty-lines 'folded)

lisp/org-clock.el

@@ -55,7 +55,6 @@
 (defvar org-frame-title-format-backup nil)
 (defvar org-state)
 (defvar org-link-bracket-re)
-(defvar org-time-stamp-formats)
 
 (defgroup org-clock nil
   "Options concerning clocking working time in Org mode."
@@ -2388,7 +2387,7 @@ have priority."
 	      (`interactive "(Range interactively set)")
 	      (`untilnow "now"))))
       (if (not as-strings) (list start end text)
-	(let ((f (cdr org-time-stamp-formats)))
+	(let ((f (org-time-stamp-format 'with-time)))
 	  (list (and start (format-time-string f start))
 		(format-time-string f end)
 		text))))))

lisp/org-colview.el

@@ -777,9 +777,8 @@ around it."
   (when (and s (string-match (concat "^" org-ts-regexp3 "$") s))
     (let* ((time (org-parse-time-string s 'nodefaults))
 	   (active (equal (string-to-char s) ?<))
-	   (fmt (funcall (if (nth 1 time) 'cdr 'car) org-time-stamp-formats))
+	   (fmt (org-time-stamp-format (nth 1 time) (not active)))
 	   time-before time-after)
-      (unless active (setq fmt (concat "[" (substring fmt 1 -1) "]")))
       (setf (car time) (or (car time) 0))
       (setf (nth 1 time) (or (nth 1 time) 0))
       (setf (nth 2 time) (or (nth 2 time) 0))

lisp/org-compat.el

@@ -384,6 +384,8 @@ Counting starts at 1."
 (define-obsolete-function-alias 'org-string-match-p 'string-match-p "9.0")
 
 ;;;; Functions and variables from previous releases now obsolete.
+(define-obsolete-function-alias 'org-timestamp-format
+  'org-format-timestamp "Org 9.6")
 (define-obsolete-variable-alias 'org-export-before-processing-hook
   'org-export-before-processing-functions "Org 9.6")
 (define-obsolete-variable-alias 'org-export-before-parsing-hook

lisp/org-element.el

@@ -95,7 +95,6 @@
 (defvar org-property-re)
 (defvar org-src-preserve-indentation)
 (defvar org-tags-column)
-(defvar org-time-stamp-formats)
 (defvar org-todo-regexp)
 (defvar org-ts-regexp-both)
 
@@ -4033,8 +4032,7 @@ Assume point is at the beginning of the timestamp."
 	  ;; the repeater string, if any.
 	  (lambda (time activep &optional with-time-p hour-end minute-end)
 	    (let ((ts (format-time-string
-		       (funcall (if with-time-p #'cdr #'car)
-				org-time-stamp-formats)
+                       (org-time-stamp-format with-time-p)
 		       time)))
 	      (when (and hour-end minute-end)
 		(string-match "[012]?[0-9]:[0-5][0-9]" ts)

lisp/org-macro.el

@@ -368,7 +368,7 @@ Return value as a string."
 	     (not (cdr date))
 	     (eq 'timestamp (org-element-type (car date))))
 	(format "(eval (if (org-string-nw-p $1) %s %S))"
-		(format "(org-timestamp-format '%S $1)"
+		(format "(org-format-timestamp '%S $1)"
 			(org-element-copy (car date)))
 		value)
       value)))

lisp/org-pcomplete.el

@@ -70,7 +70,6 @@
 (defvar org-property-re)
 (defvar org-startup-options)
 (defvar org-tag-re)
-(defvar org-time-stamp-formats)
 (defvar org-todo-keywords-1)
 (defvar org-todo-line-regexp)
 
@@ -230,7 +229,7 @@ When completing for #+STARTUP, for example, this function returns
 
 (defun pcomplete/org-mode/file-option/date ()
   "Complete arguments for the #+DATE file option."
-  (pcomplete-here (list (format-time-string (car org-time-stamp-formats)))))
+  (pcomplete-here (list (format-time-string (org-time-stamp-format)))))
 
 (defun pcomplete/org-mode/file-option/email ()
   "Complete arguments for the #+EMAIL file option."

lisp/org.el

@@ -468,8 +468,17 @@ The time stamps may be either active or inactive.")
   "Regular expression for specifying repeated events.
 After a match, group 1 contains the repeat expression.")
 
-(defconst org-time-stamp-formats '("<%Y-%m-%d %a>" . "<%Y-%m-%d %a %H:%M>")
-  "Formats for `format-time-string' which are used for time stamps.")
+(defconst org-time-stamp-formats '("%Y-%m-%d %a" . "%Y-%m-%d %a %H:%M")
+  "Formats for `format-time-string' which are used for time stamps.
+
+The value is a cons cell containing two strings.  The `car' and `cdr'
+of the cons cell are used to format time stamps that do not and do
+contain time, respectively.
+
+Leading \"<\"/\"[\" and trailing \">\"/\"]\" pair will be stripped
+from the format strings.
+
+Also, see `org-time-stamp-format'.")
 
 ;;;; Clock and Planning
 
@@ -2416,22 +2425,48 @@ To turn this on on a per-file basis, insert anywhere in the file:
 (make-variable-buffer-local 'org-display-custom-times)
 
 (defcustom org-time-stamp-custom-formats
-  '("<%m/%d/%y %a>" . "<%m/%d/%y %a %H:%M>") ; american
-  "Custom formats for time stamps.  See `format-time-string' for the syntax.
+  '("%m/%d/%y %a" . "%m/%d/%y %a %H:%M") ; american
+  "Custom formats for time stamps.
+
+See `format-time-string' for the syntax.
+
 These are overlaid over the default ISO format if the variable
 `org-display-custom-times' is set.  Time like %H:%M should be at the
 end of the second format.  The custom formats are also honored by export
-commands, if custom time display is turned on at the time of export."
-  :group 'org-time
-  :type 'sexp)
+commands, if custom time display is turned on at the time of export.
 
-(defun org-time-stamp-format (&optional long inactive)
-  "Get the right format for a time string."
-  (let ((f (if long (cdr org-time-stamp-formats)
-	     (car org-time-stamp-formats))))
-    (if inactive
-	(concat "[" (substring f 1 -1) "]")
-      f)))
+Leading \"<\" and trailing \">\" pair will be stripped from the format
+strings."
+  :group 'org-time
+  :type '(cons string string))
+
+(defun org-time-stamp-format (&optional with-time inactive custom)
+  "Get timestamp format for a time string.
+
+The format is based on `org-time-stamp-formats' (if CUSTOM is nil) or or
+`org-time-stamp-custom-formats' (if CUSTOM if non-nil).
+
+When optional argument WITH-TIME is non-nil, the timestamp will contain
+time.
+
+When optional argument INACTIVE is nil, format active timestamp.
+When `no-brackets', strip timestamp brackets.
+Otherwise, format inactive timestamp."
+  (let ((format (funcall
+                 (if with-time #'cdr #'car)
+                 (if custom
+                     org-time-stamp-custom-formats
+                   org-time-stamp-formats))))
+    ;; Strip brackets, if any.
+    (when (or (and (string-prefix-p "<" format)
+                   (string-suffix-p ">" format))
+              (and (string-prefix-p "[" format)
+                   (string-suffix-p "]" format)))
+      (setq format (substring format 1 -1)))
+    (pcase inactive
+      (`no-brackets format)
+      (`nil (concat "<" format ">"))
+      (_ (concat "[" format "]")))))
 
 (defcustom org-deadline-warning-days 14
   "Number of days before expiration during which a deadline becomes active.
@@ -13733,23 +13768,20 @@ user."
       (save-excursion
 	(end-of-line 1)
 	(while (not (equal (buffer-substring
-			    (max (point-min) (- (point) 4)) (point))
-			   "    "))
+			  (max (point-min) (- (point) 4)) (point))
+			 "    "))
 	  (insert " ")))
       (let* ((ans (concat (buffer-substring (line-beginning-position)
                                             (point-max))
 			  " " (or org-ans1 org-ans2)))
 	     (org-end-time-was-given nil)
 	     (f (org-read-date-analyze ans org-def org-defdecode))
-	     (fmts (if org-display-custom-times
-		       org-time-stamp-custom-formats
-		     org-time-stamp-formats))
-	     (fmt (if (or org-with-time
-			  (and (boundp 'org-time-was-given) org-time-was-given))
-		      (cdr fmts)
-		    (car fmts)))
+	     (fmt (org-time-stamp-format
+                   (or org-with-time
+                       (and (boundp 'org-time-was-given) org-time-was-given))
+                   org-read-date-inactive
+                   org-display-custom-times))
 	     (txt (format-time-string fmt (org-encode-time f)))
-	     (txt (if org-read-date-inactive (concat "[" (substring txt 1 -1) "]") txt))
 	     (txt (concat "=> " txt)))
 	(when (and org-end-time-was-given
 		   (string-match org-plain-time-of-day-regexp txt))
@@ -14078,9 +14110,8 @@ PRE and POST are optional strings to be inserted before and after the
 stamp.
 The command returns the inserted time stamp."
   (org-fold-core-ignore-modifications
-    (let ((fmt (funcall (if with-hm 'cdr 'car) org-time-stamp-formats))
+    (let ((fmt (org-time-stamp-format with-hm inactive))
 	  stamp)
-      (when inactive (setq fmt (concat "[" (substring fmt 1 -1) "]")))
       (insert-before-markers-and-inherit (or pre ""))
       (when (listp extra)
         (setq extra (car extra))
@@ -14125,11 +14156,10 @@ The command returns the inserted time stamp."
 	(setq off (- (match-end 0) (match-beginning 0)))))
     (setq end (- end off))
     (setq with-hm (and (nth 1 t1) (nth 2 t1))
-	  tf (funcall (if with-hm 'cdr 'car) org-time-stamp-custom-formats)
+	  tf (org-time-stamp-format with-hm 'no-brackets 'custom)
 	  time (org-fix-decoded-time t1)
 	  str (org-add-props
-		  (format-time-string
-		   (substring tf 1 -1) (org-encode-time time))
+		  (format-time-string tf (org-encode-time time))
 		  nil 'mouse-face 'highlight))
     (put-text-property beg end 'display str)))
 
@@ -19729,7 +19759,7 @@ Otherwise, use its start."
   "Non-nil when TIMESTAMP has a time specified."
   (org-element-property :hour-start timestamp))
 
-(defun org-timestamp-format (timestamp format &optional end utc)
+(defun org-format-timestamp (timestamp format &optional end utc)
   "Format a TIMESTAMP object into a string.
 
 FORMAT is a format specifier to be passed to
@@ -19790,13 +19820,13 @@ it has a `diary' type."
   (let ((type (org-element-property :type timestamp)))
     (if (or (not org-display-custom-times) (eq type 'diary))
 	(org-element-interpret-data timestamp)
-      (let ((fmt (funcall (if (org-timestamp-has-time-p timestamp) #'cdr #'car)
-			  org-time-stamp-custom-formats)))
+      (let ((fmt (org-time-stamp-format
+                  (org-timestamp-has-time-p timestamp) nil 'custom)))
 	(if (and (not boundary) (memq type '(active-range inactive-range)))
-	    (concat (org-timestamp-format timestamp fmt)
+	    (concat (org-format-timestamp timestamp fmt)
 		    "--"
-		    (org-timestamp-format timestamp fmt t))
-	  (org-timestamp-format timestamp fmt (eq boundary 'end)))))))
+		    (org-format-timestamp timestamp fmt t))
+	  (org-format-timestamp timestamp fmt (eq boundary 'end)))))))
 
 ;;; Other stuff
 

lisp/ox-odt.el

@@ -920,7 +920,7 @@ See `org-odt--build-date-styles' for implementation details."
   (let* ((format-timestamp
 	  (lambda (timestamp format &optional end utc)
 	    (if timestamp
-		(org-timestamp-format timestamp format end utc)
+		(org-format-timestamp timestamp format end utc)
 	      (format-time-string format nil utc))))
 	 (has-time-p (or (not timestamp)
 			 (org-timestamp-has-time-p timestamp)))
@@ -936,14 +936,8 @@ See `org-odt--build-date-styles' for implementation details."
 	     ;; don't bother about formatting the date contents to be
 	     ;; compatible with "OrgDate1" and "OrgDateTime" styles.  A
 	     ;; simple Org-style date should suffice.
-	     (date (let* ((formats
-			   (if org-display-custom-times
-			       (cons (substring
-				      (car org-time-stamp-custom-formats) 1 -1)
-				     (substring
-				      (cdr org-time-stamp-custom-formats) 1 -1))
-			     '("%Y-%m-%d %a" . "%Y-%m-%d %a %H:%M")))
-			  (format (if has-time-p (cdr formats) (car formats))))
+	     (date (let ((format (org-time-stamp-format
+                                  has-time-p 'no-brackets 'custom)))
 		     (funcall format-timestamp timestamp format end)))
 	     (repeater (let ((repeater-type (org-element-property
 					     :repeater-type timestamp))
@@ -1422,8 +1416,10 @@ original parsed data.  INFO is a plist holding export options."
 	 ;; value before moving on to temp-buffer context down below.
 	 (custom-time-fmts
 	  (if org-display-custom-times
-	      (cons (substring (car org-time-stamp-custom-formats) 1 -1)
-		    (substring (cdr org-time-stamp-custom-formats) 1 -1))
+              (cons (org-time-stamp-format
+                     nil 'no-brackets 'custom)
+                    (org-time-stamp-format
+                     'with-time 'no-brackets 'custom))
 	    '("%Y-%M-%d %a" . "%Y-%M-%d %a %H:%M"))))
     (with-temp-buffer
       (insert-file-contents

lisp/ox.el

@@ -4196,7 +4196,7 @@ meant to be translated with `org-export-data' or alike."
 	  ((and fmt
 		(not (cdr date))
 		(eq (org-element-type (car date)) 'timestamp))
-	   (org-timestamp-format (car date) fmt))
+	   (org-format-timestamp (car date) fmt))
 	  (t date))))
 
 

testing/lisp/test-org.el

@@ -8279,19 +8279,19 @@ CLOSED: %s
   (should-not (org-get-repeat "<2012-03-29 Thu 16:40>")))
 
 (ert-deftest test-org/timestamp-format ()
-  "Test `org-timestamp-format' specifications."
+  "Test `org-format-timestamp' specifications."
   ;; Regular test.
   (should
    (equal
     "2012-03-29 16:40"
     (org-test-with-temp-text "<2012-03-29 Thu 16:40>"
-      (org-timestamp-format (org-element-context) "%Y-%m-%d %R"))))
+      (org-format-timestamp (org-element-context) "%Y-%m-%d %R"))))
   ;; Range end.
   (should
    (equal
     "2012-03-29"
     (org-test-with-temp-text "[2011-07-14 Thu]--[2012-03-29 Thu]"
-      (org-timestamp-format (org-element-context) "%Y-%m-%d" t)))))
+      (org-format-timestamp (org-element-context) "%Y-%m-%d" t)))))
 
 (ert-deftest test-org/timestamp-split-range ()
   "Test `org-timestamp-split-range' specifications."