Derek Chen-Becker <[email protected]> writes:

> I took a look at the Emacs commit hook scripts and I didn't see anything
> that looked unusual, although I'm also not familiar enough to say for
> certain. I went ahead and created an additional patch for those scripts
> along with some minor tweaks to refer to the correct CONTRIBUTE.org file
> and to add a check for two spaces after sentences. I also added a new
> regexp for the numeric value check and to construct the cookie regexp for
> the main patch. Looking forward to feedback.

Thanks! Although I am not sure if asking people to install hooks
manually is going to work. In Emacs, hooks are installed as a part of
autoconf.sh. We may need to copy that logic into our makefiles. I doubt
that people will install the hooks in practice otherwise.

Also, we should not say that the new hooks are "a part of Emacs". They
are not. Only a subset of files in Org repository are actually a part of
Emacs.

> -(defvar org-priority-regexp ".*?\\(\\[#\\([A-Z0-9]+\\)\\] ?\\)"
> +(defvar org-priority-numeric-value-regexp "[0-9]\\|[1-5][0-9]\\|6[0-4]"

I am not sure if this is the best. A more logical way would be defining
a full regexp matching both A-Z and numeric value.

> +  "Regular expression matching valid numeric priority values.
> +The priority value must be an integer value in the range 0 through 64.")
> +
> +(defvar org-priority-regexp
> +  (format ".*?\\(\\[#\\([A-Z]\\|%s\\)\\] ?\\)" 
> org-priority-numeric-value-regexp)
> +             (let* ((msg (format "Priority %s-%s, SPC to remove: "
> +                                 (org-priority-to-string 
> org-priority-highest)
> +                                 (org-priority-to-string 
> org-priority-lowest)))
> +                    ;; Read input as a string if the current high/low range 
> could
> +                    ;; be two digits. Otherwise, we can just read a single
> +                    ;; character and save the user from pressing <enter>
> +                    (s (if (and is-numeric-priority
> +                                (>= org-priority-lowest 10))
> +                           (read-string msg)
> +                         (char-to-string (read-char-exclusive msg)))))
> +               ;; Space is a special value indicating removal. For numeric
> +               ;; priorities, parse the numeric value or ensure an upper case
> +               ;; character (and convert back to a character for validation)
> +               (if (string-equal s " ")
> +                   ?\s
> +                 (if is-numeric-priority
> +                     (progn
> +                       ;; Validate the input string to ensure it's a valid 
> value because
> +                       ;; string-to-number returns zero for a non-numeric 
> string, which could
> +                       ;; be a valid priority
> +                       (unless (string-match-p 
> org-priority-numeric-value-regexp s)
> +                         (user-error "Priority must be a number between `%s' 
> and `%s'"
> +                                  (org-priority-to-string 
> org-priority-highest)
> +                                     (org-priority-to-string 
> org-priority-lowest)))
> +                       (string-to-number s))
> +                   ;; Validation of non-numerics is handled next
> +                   (string-to-char (upcase s)))))))

Here, read-char-exclusive can yield something like ?ы, which may also
need to be captured. So, may as well do the check using the full regexp.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Reply via email to