[PATCH] org-get-cursor-date regexp patch

[PATCH] org-get-cursor-date regexp patch

[No Wayman]

[-- Attachment #1: Type: text/plain, Size: 360 bytes --]


The regular expression in `org-get-cursor-date' assumes the time 
grid string will have two digits in the hour portion of the time 
strng.
However, the grid time string does not always have two digits. For 
example:

" 8:00......"

The attached patch accounts for this and uses the rx macro to 
communicate the intent of the regular expression more clearly.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-get-cursor-date-regexp-fix --]
[-- Type: text/x-patch, Size: 1231 bytes --]

From 4724b4cc5e9600da60b465c4c2f1968b75c7c31d Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
Date: Sun, 2 Aug 2020 14:42:34 -0400
Subject: [PATCH] org.el: (org-get-cursor-date): Fix regular expression

* lisp/org.el Fix regular expression.
---
 lisp/org.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index ee8be256d..37136cc48 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18728,10 +18728,11 @@ If WITH-TIME is non-nil, returns the time of the event at point (in
 the agenda) or the current time of the day; otherwise returns the
 earliest time on the cursor date that Org treats as that date
 (bearing in mind `org-extend-today-until')."
-  (let (date day defd tp hod mod)
+  (let ((hhmm-regexp (rx (seq (group (** 1 2 digit)) ":" (group (= 2 digit)))))
+	date day defd tp hod mod)
     (when with-time
       (setq tp (get-text-property (point) 'time))
-      (when (and tp (string-match "\\([0-9][0-9]\\):\\([0-9][0-9]\\)" tp))
+      (when (and tp (string-match hhmm-regexp tp))
 	(setq hod (string-to-number (match-string 1 tp))
 	      mod (string-to-number (match-string 2 tp))))
       (or tp (let ((now (decode-time)))
-- 
2.27.0

Re: [PATCH] org-get-cursor-date regexp patch

[Kyle Meyer]

Thanks for the patch.

No Wayman writes:

> The regular expression in `org-get-cursor-date' assumes the time 
> grid string will have two digits in the hour portion of the time 
> strng.
> However, the grid time string does not always have two digits. For 
> example:
>
> " 8:00......"

Makes sense.  IMO it'd be nice to see something along the lines of this
explanation in the commit message itself.

> The attached patch accounts for this and uses the rx macro to 
> communicate the intent of the regular expression more clearly.
>
> From 4724b4cc5e9600da60b465c4c2f1968b75c7c31d Mon Sep 17 00:00:00 2001
> From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
> Date: Sun, 2 Aug 2020 14:42:34 -0400
> Subject: [PATCH] org.el: (org-get-cursor-date): Fix regular expression
>
> * lisp/org.el Fix regular expression.

The function name is missing here:

    * lisp/org.el (org-get-cursor-date): ...

> ---
>  lisp/org.el | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/lisp/org.el b/lisp/org.el
> index ee8be256d..37136cc48 100644
> --- a/lisp/org.el
> +++ b/lisp/org.el
> @@ -18728,10 +18728,11 @@ If WITH-TIME is non-nil, returns the time of the event at point (in
>  the agenda) or the current time of the day; otherwise returns the
>  earliest time on the cursor date that Org treats as that date
>  (bearing in mind `org-extend-today-until')."
> -  (let (date day defd tp hod mod)
> +  (let ((hhmm-regexp (rx (seq (group (** 1 2 digit)) ":" (group (= 2 digit)))))
> +	date day defd tp hod mod)
>      (when with-time
>        (setq tp (get-text-property (point) 'time))
> -      (when (and tp (string-match "\\([0-9][0-9]\\):\\([0-9][0-9]\\)" tp))
> +      (when (and tp (string-match hhmm-regexp tp))
>  	(setq hod (string-to-number (match-string 1 tp))
>  	      mod (string-to-number (match-string 2 tp))))
>        (or tp (let ((now (decode-time)))

To my eyes, the new variable doesn't add any clarity over keeping it
inline.

Also, I very much like rx and I'm okay if you want to stick with it
here, but in this particular case I find it less readable than

    \\([0-9]?[0-9]\\):\\([0-9][0-9]\\)

or the stricter

    \\([0-2]?[0-9]\\):\\([0-5][0-9]\\)

If you do keep the rx change, the outer seq is superfluous:

    (equal (rx (seq (group (** 1 2 digit)) ":" (group (= 2 digit))))
           (rx (group (** 1 2 digit)) ":" (group (= 2 digit))))      ;; => t

Re: [PATCH] org-get-cursor-date regexp patch

[No Wayman]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]


> Makes sense.  IMO it'd be nice to see something along the lines 
> of this
> explanation in the commit message itself.
>
> The function name is missing here:
>
>     * lisp/org.el (org-get-cursor-date): ...

> To my eyes, the new variable doesn't add any clarity over 
> keeping it
> inline.

Addressed in attached patch.

> Also, I very much like rx and I'm okay if you want to stick with 
> it
> here, but in this particular case I find it less readable than
>
>     \\([0-9]?[0-9]\\):\\([0-9][0-9]\\)
>
> or the stricter
>
>     \\([0-2]?[0-9]\\):\\([0-5][0-9]\\)

Fair enough. My preference for how the regular expression is 
notated does not matter so long as it is the correct expression.
I've inlined your second, stricter regular expression.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: org-get-cursor-date-regexp-revised --]
[-- Type: text/x-patch, Size: 2164 bytes --]

From cad2044829e6d3ff76c2a3504564ed5bfcc36bfb Mon Sep 17 00:00:00 2001
From: Nicholas Vollmer <iarchivedmywholelife@gmail.com>
Date: Sun, 2 Aug 2020 14:42:34 -0400
Subject: [PATCH] org.el: (org-get-cursor-date): Fix regular expression

* lisp/org.el (org-get-cursor-date): Fix regular expression.

Previous regular expression assumed the time grid string will have two
digits in the hour portion of the time string.  However, the time grid
string does not always have two digits.  For example:

" 8:00......"
---
 lisp/org.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index ee8be256d..f8dbb6307 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -18731,22 +18731,22 @@ earliest time on the cursor date that Org treats as that date
   (let (date day defd tp hod mod)
     (when with-time
       (setq tp (get-text-property (point) 'time))
-      (when (and tp (string-match "\\([0-9][0-9]\\):\\([0-9][0-9]\\)" tp))
+      (when (and tp (string-match "\\([0-2]?[0-9]\\):\\([0-5][0-9]\\)" tp))
 	(setq hod (string-to-number (match-string 1 tp))
-	      mod (string-to-number (match-string 2 tp))))
+              mod (string-to-number (match-string 2 tp))))
       (or tp (let ((now (decode-time)))
-	       (setq hod (nth 2 now)
-		     mod (nth 1 now)))))
+               (setq hod (nth 2 now)
+                     mod (nth 1 now)))))
     (cond
      ((eq major-mode 'calendar-mode)
       (setq date (calendar-cursor-to-date)
-	    defd (encode-time 0 (or mod 0) (or hod org-extend-today-until)
-			      (nth 1 date) (nth 0 date) (nth 2 date))))
+            defd (encode-time 0 (or mod 0) (or hod org-extend-today-until)
+                              (nth 1 date) (nth 0 date) (nth 2 date))))
      ((eq major-mode 'org-agenda-mode)
       (setq day (get-text-property (point) 'day))
       (when day
 	(setq date (calendar-gregorian-from-absolute day)
-	      defd (encode-time 0 (or mod 0) (or hod org-extend-today-until)
+              defd (encode-time 0 (or mod 0) (or hod org-extend-today-until)
 				(nth 1 date) (nth 0 date) (nth 2 date))))))
     (or defd (current-time))))
 
-- 
2.28.0

Re: [PATCH] org-get-cursor-date regexp patch

[Kyle Meyer]

No Wayman writes:

> Addressed in attached patch.

Applied (ab9b14a80), dropping the unrelated whitespace changes that were
introduced in the updated patch.