emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
* [PATCH] org-get-cursor-date regexp patch
@ 2020-08-02 18:54 No Wayman
  2020-08-10  4:57 ` Kyle Meyer
  0 siblings, 1 reply; 4+ messages in thread
From: No Wayman @ 2020-08-02 18:54 UTC (permalink / raw)
  To: emacs-orgmode, 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


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

* Re: [PATCH] org-get-cursor-date regexp patch
  2020-08-02 18:54 [PATCH] org-get-cursor-date regexp patch No Wayman
@ 2020-08-10  4:57 ` Kyle Meyer
  2020-08-10  5:58   ` No Wayman
  0 siblings, 1 reply; 4+ messages in thread
From: Kyle Meyer @ 2020-08-10  4:57 UTC (permalink / raw)
  To: No Wayman; +Cc: emacs-orgmode

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


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

* Re: [PATCH] org-get-cursor-date regexp patch
  2020-08-10  4:57 ` Kyle Meyer
@ 2020-08-10  5:58   ` No Wayman
  2020-08-11  0:43     ` Kyle Meyer
  0 siblings, 1 reply; 4+ messages in thread
From: No Wayman @ 2020-08-10  5:58 UTC (permalink / raw)
  To: Kyle Meyer; +Cc: emacs-orgmode

[-- 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


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

* Re: [PATCH] org-get-cursor-date regexp patch
  2020-08-10  5:58   ` No Wayman
@ 2020-08-11  0:43     ` Kyle Meyer
  0 siblings, 0 replies; 4+ messages in thread
From: Kyle Meyer @ 2020-08-11  0:43 UTC (permalink / raw)
  To: No Wayman; +Cc: emacs-orgmode

No Wayman writes:

> Addressed in attached patch.

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


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

end of thread, other threads:[~2020-08-11  0:44 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-02 18:54 [PATCH] org-get-cursor-date regexp patch No Wayman
2020-08-10  4:57 ` Kyle Meyer
2020-08-10  5:58   ` No Wayman
2020-08-11  0:43     ` Kyle Meyer

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

	https://git.savannah.gnu.org/cgit/emacs/org-mode.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).