Re: [patch] priorities range reversed

[Bastien Guerry]

Hi Joe,

Bastien  writes:

> See the docstring of `org-priority-highest':

I'm discarding this patch right now - feel free to submit a bug report
if there is something I missed.

Thanks,

-- 
 Bastien

Re: [patch] priorities range reversed

[Bastien]

Hi Joe,

Joe Corneli via "General discussions about Org-mode."
 writes:

> In the case of numeric priorities [#1] [#9] and so on, there is a test
> that is reversed in org.el.  

See the docstring of `org-priority-highest':

  The highest priority of TODO items.
  
  A character like ?A, ?B, etc., or a numeric value like 1, 2, etc.
  
  The default is the character ?A, which is 65 as a numeric value.
  
  If you set ‘org-priority-highest’ to a numeric value inferior to
  65, Org assumes you want to use digits for the priority cookie.
  If you set it to >=65, Org assumes you want to use alphabetical
  characters.
  
  In both cases, the value of ‘org-priority-highest’ must be
  smaller than ‘org-priority-lowest’: for example, if "A" is the
  highest priority, it is smaller than the lowest "C" priority:
  65 < 67.

Are you sure the numeric value for `org-priority-highest' is smaller
than that of `org-priority-lowest'?

For numeric priorities, [#1] is always "higher" than [#2].

If I'm not wrong and if you think we can throw a more useful error
earlier to the useful, could you provide a patch for this?

Let me know if I'm missing something, thanks,

-- 
 Bastien

Re: [patch] priorities range reversed

[Timothy]

Hi Joe,

Thanks for looking into this and figuring out a patch. I’ve just taken a peek at
your patches, and I have two minor comments:
⁃ Why use saved-position instead of save-excursion
⁃ I’m think ascii versions of ② et. al would be preferable

Also, I notice that your patches are just diffs and don’t have a commit message.
Are you planning on writing them later?

Joe Corneli via “General discussions about Org-mode.”  
writes:

> Actually, there some bigger problems with the function as well…
>
> — It didn’t update the priority cookie properly when one had been set before
>
> * [#1] Exercise: update to [#2]
>
> — After fixing that, I noticed that the included save-excursion doesn’t work
>   
> ()
>
> So, here’s a more comprehensive patch, including the previous changes
> (in order to support numeric priorities) and then adjusting the
> update/insert logic so that it makes more sense.

All the best,
Timothy

Re: [patch] priorities range reversed

[General discussions about Org-mode.]

Actually, there some bigger problems with the function as well...

— It didn’t update the priority cookie properly when one had been set before

* [#1] Exercise: update to [#2]

— After fixing that, I noticed that the included save-excursion doesn’t work
  
(https://emacs.stackexchange.com/questions/7574/why-save-excursion-doesnt-save-point-position)

So, here’s a more comprehensive patch, including the previous changes
(in order to support numeric priorities) and then adjusting the
update/insert logic so that it makes more sense.

I did not adjust the indentation, so that you can more clearly see which
lines were changed.  Inline comments in the patch explain what’s going
on.

diff --git a/lisp/org.el b/lisp/org.el
index ce68f4692..69e333c84 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11305,8 +11305,9 @@ or a character."
   (user-error "Priority commands are disabled"))
 (setq action (or action 'set))
 (let ((nump (< org-priority-lowest 65))
+  (saved-position (point))
 	  current new news have remove)
-  (save-excursion
+
 	(org-back-to-heading t)
 	(when (looking-at org-priority-regexp)
 	  (let ((ms (match-string 2)))
@@ -11337,7 +11338,7 @@ or a character."
 		 (= (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 (> (upcase new) org-priority-highest) (< (upcase new) org-priority-lowest))
 		 (user-error
 		  (if nump
 		  "Priority must be between `%s' and `%s'"
@@ -11364,8 +11365,8 @@ or a character."
 			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 (> (upcase new) org-priority-highest)
+		  (< (upcase new) org-priority-lowest))
 	  (if (and (memq action '(up down))
 		   (not have) (not (eq last-command this-command)))
 	  ;; `new' is from default priority
@@ -11377,12 +11378,26 @@ or a character."
 	;; Numerical priorities are limited to 64, beyond that number,
 	;; assume the priority cookie is a character.
 	(setq news (if (> new 64) (format "%c" new) (format "%s" new)))
+;; Actually setting the priority isn’t so straightforward
+;; There are several cases
 	(if have
+;; ① have and remove
 	(if remove
 		(replace-match "" t t nil 1)
-	  (replace-match news t t nil 2))
+  ;; ② have and not remove, i.e., update
+	  (let ((case-fold-search nil)) (looking-at org-priority-regexp))
+  (if (match-end 2)
+		  (progn
+		(goto-char (match-end 2))
+(delete-region (match-beginning 2) (match-end 2))
+		(insert news))
+	(goto-char (match-end 1))
+	(insert "[#" news "] "))
+  )
+  ;; ③ don’t have and remove — nonsense
 	  (if remove
 	  (user-error "No priority cookie found in line")
+;; ④ don’t have and not remove, i.e., insert
 	(let ((case-fold-search nil)) (looking-at org-todo-line-regexp))
 	(if (match-end 2)
 		(progn
@@ -11390,7 +11405,8 @@ or a character."
 		  (insert " [#" news "]"))
 	  (goto-char (match-beginning 3))
 	  (insert "[#" news "] "
-	(org-align-tags))
+	(org-align-tags)
+  (goto-char saved-position)
   (if remove
 	  (message "Priority removed")
 	(message "Priority of current item set to %s" news)
diff --git a/lisp/org.el b/lisp/org.el
index ce68f4692..69e333c84 100644
--- a/lisp/org.el
+++ b/lisp/org.el
@@ -11305,8 +11305,9 @@ or a character."
   (user-error "Priority commands are disabled"))
 (setq action (or action 'set))
 (let ((nump (< org-priority-lowest 65))
+  (saved-position (point))
 	  current new news have remove)
-  (save-excursion
+
 	(org-back-to-heading t)
 	(when (looking-at org-priority-regexp)
 	  (let ((ms (match-string 2)))
@@ -11337,7 +11338,7 @@ or a character."
 		 (= (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 (> (upcase new) org-priority-highest) (< (upcase new) org-priority-lowest))
 		 (user-error
 		  (if nump
 		  "Priority must be between `%s' and `%s'"
@@ -11364,8 +11365,8 @@ or a character."
 			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 (> (upcase new) org-priority-highest)
+		  (< (upcase new) org-priority-lowest))
 	  (if (and (memq action '(up down))
 		   (not have) (not (eq last-command this-command)))
 	  ;; `new' is from default priority
@@ -11377,12 +11378,26 @@ or a character."
 	;; Numerical priorities are limited to 64, beyond that number,
 	;;