minad / consult

:mag: consult.el - Consulting completing-read
GNU General Public License v3.0
1.12k stars 98 forks source link

Rewrite `consult-org--annotate` #982

Closed 45mg closed 1 month ago

45mg commented 1 month ago

See #981.

Small demonstration of annotation alignment into columns:

20240323_15h42m55s_grim Not aligned (current behavior)

20240323_15h43m07s_grim Aligned (proposed behavior)

Note that the alignment only happens once the user selects another candidate.

minad commented 1 month ago

Okay, thanks. I think we can do better with the buffer name and attach it to the candidate directly, see 8943a4ae7bb46182709dd84ba7cf429e821dbe8a and e222aacb656161233931c4ff27d7625f31f3aaf9. Maybe these changes are also helpful for you?

The reason why I did not align the columns in the first place is that headings often do not contain TODO keywords or priorities. However if a priority is present, there might also be a TODO keyword? We could then use some static aligment, e.g., simply padding the keyword column to 6 or 8 characters, since keywords are usually short. What do you think? It seems a bit too complicated to dynamically align the columns as you did in the patch here. Alternatively we could precompute the length of all todo keywords and then use that.

45mg commented 1 month ago

I think we can do better with the buffer name and attach it to the candidate directly, see https://github.com/minad/consult/commit/8943a4ae7bb46182709dd84ba7cf429e821dbe8a and https://github.com/minad/consult/commit/e222aacb656161233931c4ff27d7625f31f3aaf9. Maybe these changes are also helpful for you?

Yes, that does seem a lot more elegant :)

We could then use some static aligment, e.g., simply padding the keyword column to 6 or 8 characters, since keywords are usually short.

Usually, but it might be best not to assume so, since the user can make them as long as they want. And on the other hand, a fixed 6-8 char padding would be annoying to users who've taken the effort to come up with their own 3-4 character todo keyword names.

Also, it's always better not to waste horizontal screen space. A personal example - I use Emacs on an Android phone, where my frame width is around 60 characters to begin with. In such cases every extra bit of padding really counts.

Alternatively we could precompute the length of all todo keywords and then use that.

I think this would be even more complicated, because there is no easy way to get the list of all todo keywords in use. The closest thing I know of is the buffer-local org-todo-keywords-1 that gives you the todo keywords in that buffer. (Org lets you define per-file todo keyword sets, so I guess it wouldn't make sense for it to try to track all the keywords across all files.)

In the end, we'd probably end up dynamically building up a list of todo keywords in consult-org--headings... not much better than the situation here.

minad commented 1 month ago

To be honest, I am leaning to keep things just as they are. I don't see a reason to add complications in order to align keywords and priorities. In the Org text files itself the TODO keywords and priorities are also usually not aligned.

In the end, we'd probably end up dynamically building up a list of todo keywords in consult-org--headings... not much better than the situation here.

This would be better and simpler, since we already have the keywords in consult-org--headings. For example we do the same in consult-flymake--candidates. Still, not sure if its worth it.

45mg commented 1 month ago

Actually, the only reason I cared enough about the alignment to write this code is because my version of consult-org--annotate also lets me put the filename after the priority cookie (because if they were prefixed in consult-org-agenda-multi, then typing a file name would instead match all the headings from that file). The misalignment of filenames is a lot more irritating than the misalignment of priority cookies - 20240323_22h30m13s_grim

So if you don't want to adopt that bit of my code, then I guess it might not be worth it.

minad commented 1 month ago

The misalignment of filenames is a lot more irritating than the misalignment of priority cookies

Indeed.

So if you don't want to adopt that bit of my code, then I guess it might not be worth it.

Yes, I am not fond of the code which adds the file name annotation. I don't think that using an annotation would be a bad choice. It is just that I've already made a different choice, putting the file name in front of the candidates (visible if vertico-group-format=nil). The advantage of having the file as part of the candidate is that we can match on it. (To be precise, the situation regarding annotation matching has also changed recently, with the addition of the orderless-annotation matcher. orderless-kwd.el provides even more matchers, :ann and :grp...)

minad commented 1 month ago

I guess we can close this then? At least should it be easier to write a custom annotator given https://github.com/minad/consult/commit/8943a4ae7bb46182709dd84ba7cf429e821dbe8a. If you have some working code, please update the wiki, such that it doesn't get lost.