Re: Fwd: org-priority: allow customization of priority indicator

[Ihor Radchenko <yantar92@posteo.net>]

drlkf <drlkf@drlkf.net> writes:

> I would like to submit a patch to allow users to change the priority
> tag's form entirely. While it has been working for my use-case i.e
> basic operations inside org buffers and org-agenda, it is not perfect
> and there are some bugs that remain when using more advanced features
> with different tag forms (mine is `_p' where `p' is `[1-9]'), that I am
> not competent enough to solve. However I believe there are no breaking
> changes if the user does not modify said tag form. If there's any
> precision I can add, please let me know.

Thanks for the patch!

In general, we do not intend to support changing basic elements of Org
syntax. So, things like your _p may remain buggy because they clash with
underscore emphasis.

However, your patch is anyway useful because it makes the Org code
cleaner and more modular. A side effect that users will be able to
change the syntax at their own risk can also be considered as somewhat
odd feature (apparently useful for you at least).

As a general feedback to the patch, I suggest moving the old and new
priority variables into org-element.el and change them to defconst
(Elisp still allows altering defconsts). When moving, rename them to use
org-element-* prefix, but keep old existing variables under defvaralias.

More comments below.

> Subject: [PATCH] org-priority: allow customization of priority indicator
>
> * org.el (org-priority): Allow the user to set the prefix and suffix
> of the priority indicator so that it have a completely different form
> for them (e.g _A instead of [#A]).

Please word the patch more along the lines of "Refactor priority
syntax". Org maintainers are not going to take an extra burden of
resolving bugs coming from arbitrary changes in property syntax.

> -      (when (string-match "\\[#\\([A-Z0-9]\\)\\] ?" tmp)
> -	(plist-put props 'priority-letter (match-string 1 tmp))
> +      (when (string-match org-priority-regexp tmp)
> +	(plist-put props 'priority-letter (match-string 3 tmp))

Do note that org-priority-regexp is not the same with the old regexp.
Maybe we can introduce something like org-element-priority-regexp that
does not do match groups. Just "[\\[#[A-Z0-9]+\\][ \t]*". Then, you can use it
here instead of employing costly lazy matching in analytic
org-priority-regexp.

> -	   (priority (and (looking-at "\\[#.\\][ \t]*")
> +	   (priority (and (looking-at (format "%s.%s[ \t]*"
> +                                              (regexp-quote org-priority-prefix)
> +                                              (regexp-quote org-priority-prefix)))

Better use org-element-priority-regexp as above.

> -	   (priority (and (looking-at "\\[#.\\][ \t]*")
> +	   (priority (and (looking-at (format "%s.%s[ \t]*"
> +                                              (regexp-quote org-priority-prefix)
> +                                              (regexp-quote org-priority-prefix)))
>  			  (progn (goto-char (match-end 0))
>  				 (aref (match-string 0) 2))))

same

> -		      "\\(?: +\\(\\[#.\\]\\)\\)?"
> +		      (format "\\(?: +\\(%s.%s\\)\\)?"
> +                              (regexp-quote org-priority-prefix)
> +                              (regexp-quote org-priority-suffix))
>  		      "\\(?: +\\(.*?\\)\\)??"
>  		      "\\(?:[ \t]+\\(:[[:alnum:]_@#%:]+:\\)\\)?"
>  		      "[ \t]*$")
>  	      org-complex-heading-regexp-format
>  	      (concat "^\\(\\*+\\)"
>  		      "\\(?: +" org-todo-regexp "\\)?"
> -		      "\\(?: +\\(\\[#.\\]\\)\\)?"
> +		      (format "\\(?: +\\(%s.%s\\)\\)?"
> +                              (regexp-quote org-priority-prefix)
> +                              (regexp-quote org-priority-suffix))

same

>  		      "\\(?: +"
>  		      ;; Stats cookies can be stuck to body.
>  		      "\\(?:\\[[0-9%%/]+\\] *\\)*"
> @@ -5764,8 +5768,10 @@ needs to be inserted at a specific position in the font-lock sequence.")
>  	  '(org-activate-code (1 'org-code t))
>  	  ;; COMMENT
>  	  (list (format
> -		 "^\\*+\\(?: +%s\\)?\\(?: +\\[#[A-Z0-9]\\]\\)? +\\(?9:%s\\)\\(?: \\|$\\)"
> +		 "^\\*+\\(?: +%s\\)?\\(?: +%s[A-Z0-9]%s\\)? +\\(?9:%s\\)\\(?: \\|$\\)"
>  		 org-todo-regexp
> +		 (regexp-quote org-priority-prefix)
> +		 (regexp-quote org-priority-suffix)

same

>  ;;;; Priorities
>  
> +(defvar org-priority-prefix "[#"
> +  "Prefix to insert before a priority value to form the priority indicator.
> +It should be matched in accordance by `org-priority-regexp' in order
> +for priorities to work both-ways (inserting and extracting).")
> +
> +(defvar org-priority-suffix "]"
> +  "Suffix to insert after a priority value to end the priority indicator.
> +It should be matched in accordance by `org-priority-regexp' in order
> +for priorities to work both-ways (inserting and extracting).")
> +
>  (defvar org-priority-regexp ".*?\\(\\[#\\([A-Z0-9]+\\)\\] ?\\)"

You can use org-priority-prefix and org-priority-suffix values here.

> -      (when (looking-at "\\[#[A-Z0-9]\\]")
> +      (when (looking-at (format "%s[A-Z0-9]%s"
> +                                (regexp-quote org-priority-prefix)
> +                                (regexp-quote org-priority-suffix)))

org-element-priority-regexp

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