Hello,
Jesse Johnson <holocronwea...@gmail.com> writes:
> Since you want to comment I guess you want the patch in the e-mail
> body rather than attached. Here goes nothing.
Thank you!
It looks good. Some minor comments follow.
> From bb02cd6c00b32155c0a25f409f1bfa4160b2ddcd Mon Sep 17 00:00:00 2001
> From: Jesse Johnson <holocronwea...@gmail.com>
> Date: Sun, 22 Apr 2018 18:12:54 -0700
> Subject: [PATCH] Add priority inheritance
You need to describe here what functions or variables changed here,
e.g.,
* lisp/org-agenda (org-search-view): ...
> (defcustom org-get-priority-function nil
> - "Function to extract the priority from a string.
> -The string is normally the headline. If this is nil Org computes the
> -priority from the priority cookie like [#A] in the headline. It returns
> -an integer, increasing by 1000 for each priority level.
> -The user can set a different function here, which should take a string
> -as an argument and return the numeric priority."
> + "Function to extract the priority from current line.
> +The line is always a headline.
> +
> +If this is nil Org computes the priority of the headline from a
> +priority cookie like [#A]. It returns an integer, increasing by
You need to add two spaces after a full stop.
> +1000 for each priority level (see
> +`org-priority-char-to-integer').
> +(defcustom org-use-priority-inheritance nil
> + "Whether headline priority is inherited from its parents.
"Non-nil means headline priority is..."
> +If non-nil then the first explicit priority found when searching
> +up the headline tree applies. Thus a child headline can override
> +its parent's priority.
> +
> +When nil, explicit priorities only apply to the headline they are
> +given on.
> +
> +Regardless of setting, if no explicit priority is found then the
> +default priority is used."
> + :group 'org-priorities
> + :type 'boolean)
You need to add the following keywords:
:package-version '(Org . "9.3")
and possibly
:safe t
> +(defun org-priority-char-to-integer (character)
> + "Convert priority CHARACTER to an integer priority."
> + (* 1000 (- org-lowest-priority character)))
> +
> +(defun org-priority-integer-to-char (integer)
> + "Convert priority INTEGER to a character priority."
> + (- org-lowest-priority (/ integer 1000)))
I think those can be internal functions, so they should be renamed
`org--priority-char-to-integer' and `org--priority-integer-to-char'.
> +(defun org-get-priority (&optional pos local)
> + "Get integer priority at POS.
> +POS defaults to point. If LOCAL is non-nil priority inheritance
> +is ignored regardless of the value of
> +`org-use-priority-inheritance'. Returns nil if no priority can be
Return nil if...
> +determined at POS."
> + (save-excursion
> + (save-restriction
> + (widen)
> + (goto-char (or pos (point)))
`save-excursion' + `save-restriction' + `widen' + `goto-char' =
`org-with-point-at'
So the above would be:
(org-with-point-at pos
...)
> + (beginning-of-line)
> + (if (not (looking-at org-heading-regexp))
> + (return nil)
Indentation looks wrong, but it should be:
(unless (looking-at org-heading-regexp)
...)
> + (save-match-data
> + (cl-loop
> + (if (functionp org-get-priority-function)
> + (let ((priority (funcall org-get-priority-function)))
> + (unless (eq priority t)
> + (return priority)))
> + (when (looking-at org-priority-regexp)
> + (return (org-priority-char-to-integer
> + (string-to-char (match-string-no-properties 2))))))
> + (unless (and (not local)
> + org-use-priority-inheritance
> + (org-up-heading-safe))
> + (return (org-priority-char-to-integer
> org-default-priority)))))))))
You can write a simpler function. Please have a look at `org-get-tags'
and use `org-complex-heading-regexp' to get priority cookie.
Also could you throw in a bunch of tests in "contrib/lisp/test-org.el"
and update the manual accordingly?
Regards,
--
Nicolas Goaziou
