bug#54210: 27.1; appt-add checks for bad data only at end

bug#54210: 27.1; appt-add checks for bad data only at end

[Francesco Potortì]

appt-add asks the user for three parameters:
- time (a time string)
- message (a string)
- minutes (a number)

if the time string is badly formatted (for example if it is "8" rather than "8.00") appt-add barfs.  That's ok, but it barfs only after all three parameters have been read.  It should barf immediately after submitting the wrong parameter.

Maybe I'll find the time to look at it and propose a patch, but since I don't know, I prefer reporting it now.

bug#54210: 27.1; appt-add checks for bad data only at end

[Robert Pluim]

>>>>> On Tue, 01 Mar 2022 13:54:47 +0100, Francesco Potortì <pot@potorti.it> said:

    Francesco> appt-add asks the user for three parameters:
    Francesco> - time (a time string)
    Francesco> - message (a string)
    Francesco> - minutes (a number)

    Francesco> if the time string is badly formatted (for example if it is "8" rather
    Francesco> than "8.00") appt-add barfs.  That's ok, but it barfs only after all
    Francesco> three parameters have been read.  It should barf immediately after
    Francesco> submitting the wrong parameter.

    Francesco> Maybe I'll find the time to look at it and propose a patch, but since
    Francesco> I don't know, I prefer reporting it now.

Aha, a learning opportunity (for me :-) )

How about this (I canʼt find a `read-time-of-day', unless org has one hidden
away somewhere):

diff --git a/lisp/calendar/appt.el b/lisp/calendar/appt.el
index ebdafb438e..6a3ea7c356 100644
--- a/lisp/calendar/appt.el
+++ b/lisp/calendar/appt.el
@@ -510,10 +510,12 @@ appt-add
 Optional argument WARNTIME is an integer (or string) giving the number
 of minutes before the appointment at which to start warning.
 The default is `appt-message-warning-time'."
-  (interactive "sTime (hh:mm[am/pm]): \nsMessage: \n\
-sMinutes before the appointment to start warning: ")
-  (unless (string-match appt-time-regexp time)
-    (user-error "Unacceptable time-string"))
+  (interactive (list (let ((time (read-string "Time (hh:mm[am/pm]): ")))
+                       (unless (string-match appt-time-regexp time)
+                         (user-error "Unacceptable time-string"))
+                       time)
+                     (read-string "Message: ")
+                     (read-string "Minutes before the appointment to start warning: ")))
   (and (stringp warntime)
        (setq warntime (unless (string-equal warntime "")
                         (string-to-number warntime))))

bug#54210: 27.1; appt-add checks for bad data only at end

[Francesco Potortì]

>>>>>> On Tue, 01 Mar 2022 13:54:47 +0100, Francesco Potortì <pot@potorti.it> said:
>    Francesco> appt-add asks the user for three parameters:
>    Francesco> - time (a time string)
>    Francesco> - message (a string)
>    Francesco> - minutes (a number)
>
>    Francesco> if the time string is badly formatted (for example if it is "8" rather
>    Francesco> than "8.00") appt-add barfs.  That's ok, but it barfs only after all
>    Francesco> three parameters have been read.  It should barf immediately after
>    Francesco> submitting the wrong parameter.
>
>    Francesco> Maybe I'll find the time to look at it and propose a patch, but since
>    Francesco> I don't know, I prefer reporting it now.
>
>Aha, a learning opportunity (for me :-) )
>
>How about this (I canʼt find a `read-time-of-day', unless org has one hidden
>away somewhere):
>
>diff --git a/lisp/calendar/appt.el b/lisp/calendar/appt.el
>index ebdafb438e..6a3ea7c356 100644
>--- a/lisp/calendar/appt.el
>+++ b/lisp/calendar/appt.el
>@@ -510,10 +510,12 @@ appt-add
> Optional argument WARNTIME is an integer (or string) giving the number
> of minutes before the appointment at which to start warning.
> The default is `appt-message-warning-time'."
>-  (interactive "sTime (hh:mm[am/pm]): \nsMessage: \n\
>-sMinutes before the appointment to start warning: ")
>-  (unless (string-match appt-time-regexp time)
>-    (user-error "Unacceptable time-string"))
>+  (interactive (list (let ((time (read-string "Time (hh:mm[am/pm]): ")))
>+                       (unless (string-match appt-time-regexp time)
>+                         (user-error "Unacceptable time-string"))
>+                       time)
>+                     (read-string "Message: ")
>+                     (read-string "Minutes before the appointment to start warning: ")))
>   (and (stringp warntime)
>        (setq warntime (unless (string-equal warntime "")
>                         (string-to-number warntime))))
>

It works for me!

bug#54210: 27.1; appt-add checks for bad data only at end

[Robert Pluim]

>>>>> On Tue, 01 Mar 2022 15:31:32 +0100, Francesco Potortì <pot@potorti.it> said:

    Francesco> It works for me!

OK, Iʼll queue it up (although Iʼm running at about one push a week at
the moment, so donʼt hold your breath).

Robert
-- 

bug#54210: 27.1; appt-add checks for bad data only at end

[Robert Pluim]

tags 54210 fixed
close 54210 29.1
quit

>>>>> On Tue, 01 Mar 2022 15:40:45 +0100, Robert Pluim <rpluim@gmail.com> said:

>>>>> On Tue, 01 Mar 2022 15:31:32 +0100, Francesco Potortì <pot@potorti.it> said:
    Francesco> It works for me!

    Robert> OK, Iʼll queue it up (although Iʼm running at about one push a week at
    Robert> the moment, so donʼt hold your breath).

Closing.
Committed as 0e9420bc8f

bug#54210: 27.1; appt-add checks for bad data only at end

[Michael Heerdegen]

Robert Pluim <rpluim@gmail.com> writes:

> How about this (I canʼt find a `read-time-of-day', unless org has one hidden
> away somewhere):

Doesn't look wrong to me.

> +                       (unless (string-match appt-time-regexp time)
> +                         (user-error "Unacceptable time-string"))

I know you just more or less copied that code from the body, but I think
this can be made a `string-match-p' test.  I think that code is just
older than the existence of `string-match-p'.

Michael.