org-ql icon indicating copy to clipboard operation
org-ql copied to clipboard

Extra Priority Values (A-Z and 0-64)

Open ComedyTomedy opened this issue 3 years ago • 10 comments

Sort order is letters first, then numbers. I'm not particularly attached to that, just made a choice. Org docs don't seem to define how they interrelate when mixed.

Also renamed priority "letters" to "values" in variable names & docs. Since this added a lot of noise, here's the important change:

(let* ((priority-values (append (mapcar #'string (number-sequence 65 (+ 65 25)))
                                (mapcar #'number-to-string (number-sequence 0 64))))

ComedyTomedy avatar Nov 23 '22 21:11 ComedyTomedy

Hello,

Yes, this is probably something that needs to be addressed. But if we're going to do it, we should probably try to adhere as closely as possible to how Org does it, i.e. to be sure to accept all of the kinds of values that it accepts. Would you do a little research about what Org officially allows now? IIRC it's changed slightly in the past few major releases, or something like that.

As well, if we change what priority values are accepted, we should ensure that the priority sorter works with the new values. And it might be a good idea to revisit the priority-related tests to be sure that at least one value of each type is tested for. Would you be interested in working on those issues too?

Thanks.

alphapapa avatar Nov 24 '22 15:11 alphapapa

Thanks for the quick reply! I need to fix some silly mistakes in my code anyway - I don't use git much but it looks like I can keep tweaking my branch and the PR will be updated automatically. I'm busy for a few days but will try to fix my mistakes & get the priority sorter working next week (I thought it worked; didn't test a wide-enough range of values).

Sorting in the order 0-64, then ?A-?Z will match what org does (in Emacs 28.1 at least), both in C-c ^ and org-agenda. Emacs Docs say numeric priorities go up to 64 (which is (1- ?A), thus makes sense). They specify no minimum value, but [#0] works. Numerical values outside 0-99 don't. The function #'org-priority-to-value also does "0-64, ?A-?Z".

Org Elements API doesn't seem to agree (for now?), and for [#​0] to [#​9] returns ?0 to ?9 (which is 48-57), but for [#%] it returns ?%, so it's clearly just converting a string to a char.

I'm unfamiliar with testing / buttercup, so might not be able to add tests confidently.

ComedyTomedy avatar Nov 25 '22 00:11 ComedyTomedy

Thanks for the quick reply! I need to fix some silly mistakes in my code anyway - I don't use git much but it looks like I can keep tweaking my branch and the PR will be updated automatically. I'm busy for a few days but will try to fix my mistakes & get the priority sorter working next week (I thought it worked; didn't test a wide-enough range of values).

No problem. As you can see, there's a backlog of issues and PRs; I haven't had as much time lately to work on this package as I would like.

When you update your branch, you should generally rebase it on top of current master and then force-push. And unless your commit history matters, or unless you're separating commits for readability, feel free to squash the commits, which makes it easier to review.

Sorting in the order 0-64, then ?A-?Z will match what org does (in Emacs 28.1 at least), both in C-c ^ and org-agenda. Emacs Docs say numeric priorities go up to 64 (which is (1- ?A), thus makes sense). They specify no minimum value, but [#0] works. Numerical values outside 0-99 don't. The function #'org-priority-to-value also does "0-64, ?A-?Z".

Sounds good.

Org Elements API doesn't seem to agree (for now?), and for [#​0] to [#​9] returns ?0 to ?9 (which is 48-57), but for [#%] it returns ?%, so it's clearly just converting a string to a char.

Hm, we might want to look at that more closely. Also, Org 9.6 just came out--is that what you looked at?

I'm unfamiliar with testing / buttercup, so might not be able to add tests confidently.

No problem. Buttercup is pretty simple, and it's well-documented, but I can help you with it. Probably we'll need to tweak a few priorities in the test data file to give us something to test against.

Thanks.

alphapapa avatar Dec 09 '22 08:12 alphapapa