A couple of `org-priority' fixes

A couple of `org-priority' fixes

[Alex Giorev]

[-- Attachment #1.1: Type: text/plain, Size: 634 bytes --]

I found a couple of problems in org-priority, I think these patches fix
them.
- The first patch is truly minor, it just adds a bit to the docstring
- The problem which prompted the second patch arises when all priorities
are lowercase (you can test this by evaluating (setq org-priority-highest
?a org-priority-lowest ?z org-priority-default ?o) and then trying to set
the priority to some lowercase letter)
- The problem solved by the third patch arises when numerical priorities
are used and 32 is given as the argument, 32 being the character code of
the space character, because that character is used to signal property
removal.

[-- Attachment #1.2: Type: text/html, Size: 694 bytes --]

[-- Attachment #2: 0001-lisp-org.el-Add-remove-action-in-org-priority-docstr.patch --]
[-- Type: text/x-patch, Size: 883 bytes --]

From 4f2b192743e4f8056a1e6682dbffa3d236700b46 Mon Sep 17 00:00:00 2001
From: alexgiorev <alex.giorev@gmail.com>
Date: Sat, 5 Feb 2022 00:33:54 +0200
Subject: [PATCH 1/3] lisp/org.el: Add `remove' action in org-priority
 docstring

* lisp/org.el (org-priority): Add `remove' action in org-priority docstring

TINYCHANGE
---
 lisp/org.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org.el b/lisp/org.el
index ef8d460e1..2543498de 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11346,7 +11346,7 @@ When called interactively with a `\\[universal-argument]' prefix,
 show the priority in the minibuffer instead of changing it.
 
 When called programmatically, ACTION can be `set', `up', `down',
-or a character."
+`remove', or a character."
   (interactive "P")
   (when show
     ;; Deprecation warning inserted for Org 9.2; once enough time has
-- 
2.25.1


[-- Attachment #3: 0002-lisp-org.el-Remove-upcase-new-in-org-priority.patch --]
[-- Type: text/x-patch, Size: 1648 bytes --]

From 524ee9c37c99019b6e700c98919ebb1bd8fd57c7 Mon Sep 17 00:00:00 2001
From: alexgiorev <alex.giorev@gmail.com>
Date: Sat, 5 Feb 2022 00:43:40 +0200
Subject: [PATCH 2/3] lisp/org.el: Remove (upcase new) in `org-priority'

* lisp/org.el (org-priority): After the new priority is computed,
when checking if it is within the bounds defined by
`org-priority-highest' and `org-priority-lowest', the priority is
upcased, but this leads to an error when all priorities are lowercase
letters.

TINYCHANGE
---
 lisp/org.el | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 2543498de..757c306bc 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11391,7 +11391,7 @@ When called programmatically, ACTION can be `set', `up', `down',
 		     (= (upcase org-priority-lowest) org-priority-lowest))
 	    (setq new (upcase new)))
 	  (cond ((equal new ?\s) (setq remove t))
-		((or (< (upcase new) org-priority-highest) (> (upcase new) org-priority-lowest))
+		((or (< new org-priority-highest) (> new org-priority-lowest))
 		 (user-error
 		  (if nump
 		      "Priority must be between `%s' and `%s'"
@@ -11418,8 +11418,8 @@ When called programmatically, ACTION can be `set', `up', `down',
 			    org-priority-default
 			  (1+ org-priority-default))))))
 	 (t (user-error "Invalid action")))
-	(when (or (< (upcase new) org-priority-highest)
-		  (> (upcase new) org-priority-lowest))
+	(when (or (< new org-priority-highest)
+		  (> new org-priority-lowest))
 	  (if (and (memq action '(up down))
 		   (not have) (not (eq last-command this-command)))
 	      ;; `new' is from default priority
-- 
2.25.1


[-- Attachment #4: 0003-lisp-org.el-Fix-priority-32-removing-the-cookie-in-o.patch --]
[-- Type: text/x-patch, Size: 2271 bytes --]

From 5aea96e258b694d25aa9ebc0012f499d09b415b6 Mon Sep 17 00:00:00 2001
From: alexgiorev <alex.giorev@gmail.com>
Date: Sat, 5 Feb 2022 00:50:26 +0200
Subject: [PATCH 3/3] lisp/org.el: Fix priority 32 removing the cookie in
 org-priority

* lisp/org.el (org-priority): When numerical priorities are used, and
the caller supplies 32 as the new priority value, the priority cookie
is removed because this is the code point of the space character. This
patch fixes this so that supplying 32 will change the priority to 32.

TINYCHANGE
---
 lisp/org.el | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/lisp/org.el b/lisp/org.el
index 757c306bc..3ffddff94 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11382,21 +11382,23 @@ When called programmatically, ACTION can be `set', `up', `down',
                                (read-string msg)
                              (message msg)
                              (char-to-string (read-char-exclusive)))))
-                   (if (equal s " ") ?\s (string-to-number s)))
+                   (if (equal s " ")
+                       (progn (setq remove t) ?\s)
+                     (string-to-number s)))
 	       (progn (message "Priority %c-%c, SPC to remove: "
 			       org-priority-highest org-priority-lowest)
 		      (save-match-data
-			(setq new (read-char-exclusive)))))))
+			(setq new (read-char-exclusive)
+                              remove (= new ?\s)))))))
 	  (when (and (= (upcase org-priority-highest) org-priority-highest)
 		     (= (upcase org-priority-lowest) org-priority-lowest))
 	    (setq new (upcase new)))
-	  (cond ((equal new ?\s) (setq remove t))
-		((or (< new org-priority-highest) (> new org-priority-lowest))
-		 (user-error
-		  (if nump
-		      "Priority must be between `%s' and `%s'"
-		    "Priority must be between `%c' and `%c'")
-		  org-priority-highest org-priority-lowest))))
+          (when (or (< new org-priority-highest) (> new org-priority-lowest))
+            (user-error
+	     (if nump
+		 "Priority must be between `%s' and `%s'"
+	       "Priority must be between `%c' and `%c'")
+	     org-priority-highest org-priority-lowest)))
 	 ((eq action 'up)
 	  (setq new (if have
 			(1- current)  ; normal cycling
-- 
2.25.1

Re: A couple of `org-priority' fixes

[Ihor Radchenko]

Alex Giorev <alex.giorev@gmail.com> writes:

> I found a couple of problems in org-priority, I think these patches fix
> them.

Marking as a patch for updates.orgmode.org

Confirmed.

Best,
Ihor

Re: A couple of `org-priority' fixes

[Ihor Radchenko]

Alex Giorev <alex.giorev@gmail.com> writes:

> I found a couple of problems in org-priority, I think these patches fix
> them.

Thanks for the very clean patches and sorry for the late reply.

> - The first patch is truly minor, it just adds a bit to the docstring
> - The problem which prompted the second patch arises when all priorities
> are lowercase (you can test this by evaluating (setq org-priority-highest
> ?a org-priority-lowest ?z org-priority-default ?o) and then trying to set
> the priority to some lowercase letter)
> - The problem solved by the third patch arises when numerical priorities
> are used and 32 is given as the argument, 32 being the character code of
> the space character, because that character is used to signal property
> removal.

I have tested your patches using make test (see
https://orgmode.org/worg/org-contribute.html#first-patch) and it appears
that your patches break removing priority from a headline.

Can you please fix this issue?

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
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>