Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consult-org-heading, consult-org-agenda #276

Merged
merged 27 commits into from
Apr 24, 2021
Merged

consult-org-heading, consult-org-agenda #276

merged 27 commits into from
Apr 24, 2021

Conversation

astoff
Copy link
Contributor

@astoff astoff commented Apr 18, 2021

Possible org-related commands were already discussed in the wishlist thread. In my opinion, the two most useful actions would be jumping to an agenda item and clocking in. This pull request is a draft for those two commands.

Possible improvements:

  • Flatten the tree structure. That is, insetead of showing candidates
    * Heading 1
    ** Subheading 1.1
    * Heading 2
    
    display this as
    Heading 1
    Heading 1 > Subheading 1.1
    Heading 2
    
  • Add narrowing. One could go wild here, and I don't even know what all the possibilities are. Some ideas:
    • By TODO/DONE state
    • By priority
    • By outline level ≤ n
  • Performance: Building the candidate list could be slow, if one has lots of stuff in their agenda files. In my case it only takes a fraction of a second, but a lag is still noticeable.
  • Category: should we use consult-location here, or invent a new category to allow attaching extra information to candidates, for instance the information needed for narrowing? Also, do I get it correctly that using lists of strings instead of lists of conses is the preferred way to build a candidate list now?
  • Fontification: I think I'd rather remove the org heading faces from the candidates. I like to use larger, variable pitch faces for outline-n.
  • In consult-clock-in, it would be nice to somehow inform the user about the currently clocked task, if any. One possibility would be to show the task name in the prompt, as in Clock in (current: Heading 1): . Any other suggestions?

What do you think?

@minad
Copy link
Owner

minad commented Apr 18, 2021

I am still a bit on the line regarding org commands. While I use org and like it, it offers vast functionality and I am not always sure that new functionality is really needed inside Consult. Org already provides many fancy UIs on its own. So maybe I just feel like I am not enough of an org power user to maintain these commands and they are a better fit for a separate consult-org package?

I observed for example that the command org-set-tags-command is broken if you use an enhanced completion UI (https://github.com/minad/vertico#org-set-tags-command). I think I've also seen the tags command which fixes the upstream version proposed here. I think such command proposals should be rejected or rather fixed directly in Org.
But the commands you are proposing here are of a different quality, more useful and not just fix-ups of some upstream versions.

To address your comments:

Flatten the tree structure

I cannot tell what is better. For imenu it works well, for consult-outline it would not work well. It all depends on if you want to show the full line or only the heading. Maybe give it a try to see which works out better?

Add narrowing

I personally wouldn't overdo it with the narrowing options. Right now, narrowing is used in a very "narrow" sense only, think like in consult-buffer/consult--multi, to unhide a few candidates for example. There is no support for fancy filtering. I thought before about supporting more options for narrowing, allowing to narrow according to multiple criteria etc. But these things are in conflict with completion filtering, why would we want to add another filtering system on top of basic completion? For fancy filtering narrowing one would rather want oantolin/orderless#30.

Performance: Building the candidate list could be slow, if one has lots of stuff in their agenda files. In my case it only takes a fraction of a second, but a lag is still noticeable.

Is it possible to cache the items somehow? If it is too slow, it should not be added here. But a noticeable lag does not sound too bad, it is like consult-line on a large file.

Category: should we use consult-location here, or invent a new category to allow attaching extra information to candidates

It depends on if you want to attach Marginalia annotations or Embark actions.

for instance the information needed for narrowing?

The category is not needed for narrowing.
(There is a thought - one could write a narrowing package which enhances existing commands based on the completion category. Then one would need separate completion categories. However since narrowing is implemented via predicates it would not be so easy and could break commands. The predicates also receive the exact candidates, e.g., conses and not the candidate strings).

Also, do I get it correctly that using lists of strings instead of lists of conses is the preferred way to build a candidate list now?

Yes, that's right.

Fontification: I think I'd rather remove the org heading faces from the candidates. I like to use larger, variable pitch faces for outline-n.

This should be configurable if you use consult--buffer-substring, see consult-fontify-preserve.

In consult-clock-in, it would be nice to somehow inform the user about the currently clocked task, if any. One possibility would be to show the task name in the prompt, as in Clock in (current: Heading 1): . Any other suggestions?

Sounds good. But I am not using the clock feature right now 🤷

Sorry for not being more concrete! I will add some more comments to the code.

consult.el Outdated Show resolved Hide resolved
consult.el Outdated Show resolved Hide resolved
consult.el Outdated Show resolved Hide resolved
@astoff
Copy link
Contributor Author

astoff commented Apr 18, 2021

Org already provides many fancy UIs on its own.

True. It tends to use Magit-style menus (of course Org predates Magit, by far). In fact, C-u M-x org-clock-in provides a menu for clocking in. If you ask me, it's the wrong kind of UI for that operation. That's why I find the proposed consult-clock-in relevant.

So maybe I just feel like I am not enough of an org power user to maintain these commands and they are a better fit for a separate consult-org package?

I am by no means an Org power user, and I wouldn't want to maintain a consult-org package. Now, I would argue the 2 proposed commands are of somewhat broad appeal. I can't think of a third org-related command that would be of broad appeal.

I personally wouldn't overdo it with the narrowing options.

In this case I will not add any narrowing, at least for now.

I'll react to your other comments when I have time to work on this again. And we can also wait a bit to let the question of having or not Org-related stuff in Consult mature.

@astoff
Copy link
Contributor Author

astoff commented Apr 20, 2021

So, I think I've addressed all your code comments above. I also added narrowing; I may even have gone a bit overboard with it. But it turns out that the most useful (narrow by TODO) was the trickier to get right. Concerning flattening of the tree structure, I noticed the command would be useless without it. Lastly, I created separated consult-org-goto and consult-org-agenda commands. The former by default only lists headings in the current buffer. I think it will be clear why when you see the code.

And again: I'm not insisting that this should be merged :-). I don't like the idea of having lots of separate little packages, but I might create it if you don't want to include this file in Consult. Or maybe not...

consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
@minad
Copy link
Owner

minad commented Apr 20, 2021

Okay, I think this looks good besides a few minor nits.

consult-org.el Outdated Show resolved Hide resolved
@astoff
Copy link
Contributor Author

astoff commented Apr 21, 2021

You might want to add to the user configuration that consult-org-agenda requires org-agenda-files to be set, and consult-org-clock benefits from saving recent clock entries across Emacs sessions, which is done by calling (org-clock-persistence-insinuate) in the init file.

@minad
Copy link
Owner

minad commented Apr 21, 2021

You might want to add to the user configuration that consult-org-agenda requires org-agenda-files to be set, and consult-org-clock benefits from saving recent clock entries across Emacs sessions, which is done by calling (org-clock-persistence-insinuate) in the init file.

You mean to the readme? Feel free to add your documentation there too.

@minad minad mentioned this pull request Apr 21, 2021
20 tasks
consult-org.el Outdated Show resolved Hide resolved
@astoff
Copy link
Contributor Author

astoff commented Apr 21, 2021

You mean to the readme? Feel free to add your documentation there too.

I was going to add this, but since it's more of an org-mode hint, I'll let you decide what to do

 ;; Optionally configure Org.
 (use-package org
   :config
   ;; Add some files to your agenda, either with `C-c [` or manually:
   (setq org-agenda-files ...)
   ;; `consult-org-clock-in` offers recent clocked tasks for
   ;; selection. To remember those across Emacs sessions, use the
   ;; following:
   (org-clock-persistence-insinuate))

Otherwise, it's perhaps good to merge now :-)

@minad
Copy link
Owner

minad commented Apr 22, 2021

Okay, I will probably not add the config example to the README. Org users will know. This looks pretty good now. I took another closer look and still have some improvement proposals. Sorry for the back and forth multiple times, I guess I am a bit more critical since these are the first org commands which are going to be added.

consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
consult-org.el Outdated Show resolved Hide resolved
@astoff
Copy link
Contributor Author

astoff commented Apr 22, 2021

Now that I think more about the narrow API, I get the impression that it was thought around splitting the candidate list into disjoint collections, and one needs to do some contortions when that's not the case.

Fundamentally, the narrowing data should just be a list of predicates. But since we need a UI to select a predicate, the narrowing data should be a list of elements of the form '(character title predicate), where predicate is just a function that takes a candidate as argument.

In many cases this would seem like overkill, and for convenience one could accept

'(function (?a "Category A") (?b "Category B") ...)

as a synonym for

`((?a "Category A" ,(apply-partially function ?a)) (?b "Category B") ,(apply-partially function ?b)) ...)

or something similar using a dynamic var.

This would also make the narrowing data reusable. For instance, we could append the consult-outline narrowing data to that of the org commands, and only implement narrowing by priority and todo state in consult-org.el.

Or am I missing something?

@minad
Copy link
Owner

minad commented Apr 22, 2021

Now that I think more about the narrow API, I get the impression that it was thought around splitting the candidate list into disjoint collections, and one needs to do some contortions when that's not the case.

You are right. I am sorry for the confusion. I missed that the levels are not disjoint. Then my narrowing proposal is not better to what you have. Todo status and priorities are disjoint, but not the levels. Actually not much thought has been put into the narrow API and I prefer to not over do this in order to avoid introducing another complicated filtering system on top of the completion style as I said in my first comment here.

This would also make the narrowing data reusable. For instance, we could append the consult-outline narrowing data to that of the org commands, and only implement narrowing by priority and todo state in consult-org.el.

The scheme you describe is very similar to consult-bookmark-narrow or consult-register-narrow. However it is better to not rewrite everything as reusable predicates since then you have to always go over the whole list of predicates. I think we would not save much code by generalizing what we already have. outline and org is an example, are there more?

@astoff
Copy link
Contributor Author

astoff commented Apr 22, 2021

outline and org is an example, are there more?

Perhaps not. But whatever you had in mind in oantolin/orderless#30 would be an example, right? The basic difference between the two cases would be the responsibility of defining the categories and predicates: the caller of completing-read or the generator of the candidate list.

@minad
Copy link
Owner

minad commented Apr 22, 2021

Perhaps not. But whatever you had in mind in oantolin/orderless#30 would be an example, right?

Let's keep it simple as is for now. In oantolin/orderless#30 I had in mind to add these filtering capabilities for exisiting commands depending on the category. Btw one could also implement a narrowing package in the style of Marginalia which enhances existing commands, e.g. adds narrowing/grouping to M-x.

@protesilaos
Copy link
Sponsor

protesilaos commented Apr 22, 2021 via email

@astoff
Copy link
Contributor Author

astoff commented Apr 22, 2021

I've pushed a commit that makes the algorithms O((m+n)log(m)) — I think.

Now, hash tables aren't meant to work with markers, since they get mutated all the time. But one can define a hash table test which is okay for short-lived hash tables.

@minad
Copy link
Owner

minad commented Apr 23, 2021

I am mostly happy with this now. The question is if we want to prepend the buffer name to the candidates if we are in a multi-buffer scope. Then we can hide the buffer name again using the :title function. It is a very recent change I just made yesterday and today to consult--read, to allow the title function to transform candidates.

@astoff
Copy link
Contributor Author

astoff commented Apr 23, 2021

I really liked the grouping/title thing, and I think the default should be with titles and no prepended buffer name, at least on the vertical UIs.

But we have all information to prepend the buffer name at hand, so it should be easy to add. If it's also easy to remove, I see no harm in this.

@minad
Copy link
Owner

minad commented Apr 23, 2021

I really liked the grouping/title thing, and I think the default should be with titles and no prepended buffer name, at least on the vertical UIs.

The idea here is to add it for the filtering. If the UI supports the title function the prefix will be removed again and will be shown only in the title. I work on a patch for this now :)

@minad
Copy link
Owner

minad commented Apr 23, 2021

Okay, I added the commit. You may want to give it another try and then I will merge this.

@astoff
Copy link
Contributor Author

astoff commented Apr 24, 2021

Looks good to me!

@minad minad merged commit 9728ca0 into minad:main Apr 24, 2021
@minad
Copy link
Owner

minad commented Apr 24, 2021

Thank you for the contribution!

minad added a commit that referenced this pull request Apr 24, 2021
Co-authored-by: Daniel Mendler <mail@daniel-mendler.de>
@minad minad changed the title consult-agenda and consult-clock-in consult-org-heading, consult-org-agenda May 8, 2021
@gdindi
Copy link

gdindi commented May 21, 2021

Hi,
I think that commands allowing to navigate the clock history, jumping to the current clocked task and clocking in a task from the history would be very useful. This lacking feature is what keeps me using counsel. I clock most of my time in org-mode and I use these all the time:
counsel-org-clock-history
counsel-org-clock-goto
counsel-org-clock-context

I can help testing them if needed.
Thanks!

@minad
Copy link
Owner

minad commented May 21, 2021

@gdindi There has been a long discussion here. It is better to implement these clocking commands as Embark actions.

@gdindi
Copy link

gdindi commented May 21, 2021

Thanks for your answer. I don't understand what that means, but I will try to decode looking at what Embark does.

@astoff
Copy link
Contributor Author

astoff commented May 21, 2021

I've kept a dedicated clock-in command in my config, and I'm actually using it a lot (usually directly, more rarely as an Embark action). I don't know what these 3 variants that @gdindi mentions would be, though.

@minad (cc @oantolin) Aren't Embark actions just plain old commands? So how would an Embark clock-in action differ from a consult-org-clock-in command, and why would it make more sense for Embark to provide it?

@minad
Copy link
Owner

minad commented May 21, 2021

@astoff I meant that one could execute org-clock commands on the org headings in consult-org-heading for example. I don't know what alternative you are using right now, but if I recall correctly this org-clock-in command could be easily replicated based on consult-org-heading + plain org-clock-in in very few lines? Is this correct?

@astoff
Copy link
Contributor Author

astoff commented May 21, 2021

@astoff I meant that one could execute org-clock commands on the org headings in consult-org-heading for example.

That unfortunately doesn't work. One needs a wrapper command. FWIW, a bare-bones consult-clock-in is trivial to define, but adding bells and whistles is not.

@gdindi Jump to the current task already exists: org-clock-goto.

@minad
Copy link
Owner

minad commented May 21, 2021

FWIW, a bare-bones consult-clock-in is trivial to define, but adding bells and whistles is not.

Yes, this is what I meant. Which additional bells and whistles you would like to have?

@astoff
Copy link
Contributor Author

astoff commented May 21, 2021

The main one is a *Recent* group — precisely the thing you didn't like, implementation-wise, when I originally made the MR.

Should I maybe add the simple and the fancy versions on the wiki? I just didn't want this to mean the command will never make it into consult-org.el.

@minad
Copy link
Owner

minad commented May 21, 2021

Wiki is the right place for these commands now. I am looking for more general building blocks, for example consult-org-heading is great :)

@astoff
Copy link
Contributor Author

astoff commented May 21, 2021

@gdindi
Copy link

gdindi commented May 22, 2021

@astoff Nice. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants