Questions about a consult-notes command attempt

[hmelman]

I'm trying to use consult--multi to manage several directories of notes files. I have the following:

(defun hrm-notes-closure (dir)
  "Return a function for an hrm-notes source rooted in DIR.
The function called with no arg returns a list of filenames in DIR.
Called with an arg f opens the file in DIR."
  (lambda (&optional f)
    (if f
	(find-file (expand-file-name f dir))
      (directory-files dir))))

(defvar hrm-notes-restaurants-function (hrm-notes-closure "~/Dropbox/Restaurants/"))
(defvar hrm-notes-lectures-function (hrm-notes-closure "~/Dropbox/Lectures/"))

(defvar hrm-notes-restaurants-source
  `(:name     "Restaurants"
    :narrow   ?r
    :category file
    :face     consult-file
    :history  hrm-notes-history
    :items    ,hrm-notes-restaurants-function
    :action   ,hrm-notes-restaurants-function))

(defvar hrm-notes-lecture-source
  `(:name     "Lectures"
    :narrow   ?l
    :category file
    :face     consult-file
    :history  hrm-notes-history
    :items    ,hrm-notes-lectures-function
    :action   ,hrm-notes-lectures-function))

(defvar hrm-notes-sources
  (list 'hrm-notes-restaurants-source
	'hrm-notes-lecture-source))

(defvar hrm-notes-history nil
  "History variable for hrm-notes.")

(defun hrm-notes ()
  "Find a file in a notes directory."
  (interactive)
  (consult--multi
   hrm-notes-sources
   :prompt "Notes File: "
   :history 'hrm-notes-history))

First off, it works, but do you see any problems in the above?

I'd like to figure out how to add annotations to these. At first just the source name and then maybe dates and sizes like marginalia does for files. I couldn't get that going, even for just the simple fixed string case. I tried adding a :annotate to the source and to the consult--multi call with a function that returned ("Restaurants") but it didn't show. For file attributes I'll have another problem as the default-directory will be different. Do you have a recommendation for how to get it to the annotation function? I could make a different one for each source, or store it in a property. It wasn't clear to me if the return of :items had to be just a list of strings or if like candidates it could be a list of cons with a string and value that could be the full path.

I had tried before but was confused by how to ultimately call find-file, so the addition of the :action param definitely helped me. I think I now get how :state and "restore" works with the new description. IIRC I'd probably rename "restore" to "final" and say the final call is used to restore things if previewing has occurred and then to perform the desired action.

When I get this done I'll add it to wiki as an example and I'll give comments on consult--multi documentation.

[minad]

IIRC I'd probably rename "restore" to "final" and say the final call is used to restore things if previewing has occurred and then to perform the desired action.

Yes, I considered this and up to now I have been to lazy to change it everywhere. This is somehow "historic" since it all came out of the original preview function definition, where the restore name made more sense.

It wasn't clear to me if the return of :items had to be just a list of strings or if like candidates it could be a list of cons with a string and value that could be the full path.

It has to be a list of strings or a function returning a list of strings.

Regarding annotations, I will check if this functionality still works.

[minad]

Annotations work for me:

(nconc consult--source-buffer `(:annotate ,(lambda (cand) (format "==%S==" cand))))

[minad]

The main problem in your source definitions above is that you are using :action and :items wrong. Items should return a list of strings. Action should take the candidate and act on it, e.g., jump to it/open it. Besides that it should work what you are doing. Please report back if it you manage to get it working!

[minad]

Right, I missed this question:

For file attributes I'll have another problem as the default-directory will be different. Do you have a recommendation for how to get it to the annotation function?

You can add text properties to the candidates. These can then be extracted in the :annotate function.

To the problem mentioned by @oantolin, Marginalia annotations should work as expected. They won't work if the path is not correct or not relative to the default directory. I recommend using abbreviated or absolute paths. Or provide your own annotator.

[oantolin]

I'd suggest just changing default-directory in the minibuffer to the right place. Just add a (setq default-directory dir) to those closures.

The main problem in your source definitions above is that you are using :action and :items wrong. Items should return a list of strings. Action should take the candidate and act on it, e.g., jump to it/open it. Besides that it should work what you are doing. Please report back if it you manage to get it working!

But it does, right? Called with no arguments his closures return a list of strings, called with one argument they visit the file.

[hmelman]

But it does, right? Called with no arguments his closures return a list of strings, called with one argument they visit the file.

Yes, as I said it does work. But adding this doesn't show any annotations for me:

(defun hrm-annotate-rest (cand)
  "Annotation function for restaurants"
  (format "== Restaurant ==" cand))
  
(defvar hrm-notes-restaurants-source
  `(:name     "Restaurants"
    :narrow   ?r
    :category file
    :face     consult-file
    :history  hrm-notes-history
    :annotate hrm-annotate-rest
    :items    ,hrm-notes-restaurants-function
    :action   ,hrm-notes-restaurants-function))
  

It also doesn't work if I add this to the consult--multi call

   :annotate 'hrm-annotate-rest

[minad]

But it does, right? Called with no arguments his closures return a list of strings, called with one argument they visit the file.

Oh right, I was confused by the code and I missed this. I somehow see the point since @hmelman wants to reuse a single function. The intended design is to use different functions for everything. Maybe people prefer a different design. Sources could also be defined as a single function taking an action argument, e.g. 'annotate, 'items and so on. I intentionally implemented it like this since it is easier to replace fields of sources. You can only replace the annotation function etc. Otherwise you would have to write a wrapper function catching the action and delegating the rest to the original function.

I'd suggest just changing default-directory in the minibuffer to the right place. Just add a (setq default-directory dir) to those closures.

Sounds good, if each source has an associated default directory you can change there. But you don't mean in the context of Marginalia, right?

[minad]

@hmelman Does this work for you?

(nconc consult--source-buffer `(:annotate ,(lambda (cand) (format "==%S==" cand))))

If not my usual questions apply. Do you have the newest versions of everything? Which Emacs version and so on.

[hmelman]

I don't need to use the same function it just seemed convenient. For this command all my sources will be very similar, just differing in the directory and some names. so after I got it working with a few functions it was easy to refactor to one.

[minad]

I don't need to use the same function it just seemed convenient. For this command all my sources will be very similar, just differing in the directory and some names. so after I got it working with a few functions it was easy to refactor to one.

Sure. It is not wrong what you are doing. It just confused me since I was mentally fixated to my intended design.

[hmelman]

I just updated to latest from melpa (I think I was using yesterdays before). No, doing your nconc doesn't change the display of consult-buffer for me. Yes, I checked that consult--source-buffer was modified. Macport of Emacs 27.1.

[minad]

@hmelman Do you see the Marginalia annotations instead? Please disable marginalia-mode and recheck.

[minad]

@oantolin and I made this intrusive design decision in Marginalia that it overrides with its annotations if enabled, since it "knows more" and generally provides better annotations than the default annotators, in particular the ones provided by Emacs itself. If you use the 'file or 'buffer category, then Marginalia will set in and show its annotations. If you want your own annotator, you should use a different category.

[hmelman]

Yes, disabling marginalia-mode shows my annotations.

[minad]

I didn't suggest this immediately since I am using the light Marginalia annotators by default which do not override file and buffer. But if you use heavy annotators, then Marginalia overrides file and buffer. See marginalia-annotators-heavy.

[hmelman]

And it looks like with marginalia-mode disabled, it automatically shows the source :name value which is nice. I'm not sure what mechanism is doing that.

[hmelman]

Categories still aren't completely clear to me. I know they are defined in the completion api but are they used for anything in this whole stack other than to choose annotations?

[minad]

And it looks like with marginalia-mode disabled, it automatically shows the source :name value which is nice. I'm not sure what mechanism is doing that.

This is a consult feature. consult--multi-annotate adds the source names.

Categories still aren't completely clear to me. I know they are defined in the completion api but are they used for anything in this whole stack other than to choose annotations?

They are also used to associate Embark actions with the candidate. Furthermore the category can override the completion style, see completion-category-defaults and completion-category-overrides.

[hmelman]

More a marginalia question but I'll continue it here. I've changed my sources to use :category note and my own annotation function. I have:

(setq marginalia-annotators '(marginalia-annotators-heavy marginalia-annotators-light nil))
(define-key minibuffer-local-map (kbd "M-a") 'marginalia-cycle)

because I want to default to heavy and use M-a to cycle to light and no annotations. But I find my annotations don't show when heavy is selected. I thought changing category to note (which isn't in heavy or light, they are the default values) would mean marginalia wouldn't override my annotation function.

[oantolin]

@minad

I'd suggest just changing default-directory in the minibuffer to the right place. Just add a (setq default-directory dir) to those closures.

Sounds good, if each source has an associated default directory you can change there. But you don't mean in the context of Marginalia, right?

I did mean in the context of Marginalia. The idea was that if the default-directory setting is correct, Marginalia can use its normal file annotator, since @hmelman is using the file category and he said about the annotations that he wanted "at first just the source name and then maybe dates and sizes like marginalia does for files", so I thought he'd be OK with just reusing Marginalia's heavy file annotator.

But maybe setting the default directory in the minibuffer doesn't work, since Marginalia actually uses the value of default-directory from (buffer-window (minibuffer-selected-window)), right?

[minad]

@hmelman It seems you have found a bug with this custom annotator and marginalia heavy. I can confirm this, I am also not seeing an annotator in this case. The problem is probably the consult-multi dispatcher. I will push a fix.

[minad]

@oantolin Yes, I am sure you can somehow mess around with Marginalia to fix the directory such that local paths work. But I would not go that route it will be too brittle. The annotators are executed in the original window. So if the default-directory is correct there it should work.

[hmelman]

@oantolin I think it won't work because different candidates will have a different directory (that's the point of this command using multiple sources, collecting files from different directories). I'd need to change the default-director each time the annotator is run and I don't see how I'd do that? Unless the default marginalia file annotator got a setup hook. ;)

[oantolin]

Oh! Sorry @hmelman, I forgot you were trying to combine multiple sources. I think I'd recommend not using the file category. I would:

• make up my own name for the category
• add the directory to each candidate in a text property
• write an annotator (that uses that directory)
• write a transformer for Embark that transforms your category to file with the full path.

[minad]

@oantolin This sounds like what @hmelman is planning to do!
@hmelman I pushed a fix minad/marginalia@86c0461.

[hmelman]

Thanks, I'll get the fix when it hits melpa. Yes I've got some of it working with the dir in a text property. I'm going to play with marginalia--fields for my annotation, any thoughts about putting some of it in a public api to help people write their own annotators?

[hmelman]

I seem to be hitting something else that's slightly annoying. I'll change my closure function or a source; I check the value of the source variable and it's definitely updated. But if I run hrm-notes I don't get the new behavior. If I restart emacs and load this file I do. Is consult--multi caching the sources somehow? I see the let binding of consult--cache which I read as setting it to nil so no caching, but maybe I'm missing something? If there is sources caching I'd love a way to disable it during development.

[oantolin]

I say we bite the bullet and just call string-width on the display strings and visible substrings. And revisit this decision only i it feels slow or these width computations start showing up in profiles.

[minad]

@oantolin See above, I added a small optimization which avoids allocations for candidates without invisible and display parts, which should be most candidates.

[oantolin]

Have you read the docstring of string-width? I'd say it's a bug that it doesn't already take invisible and display properties into account:

Return width of STRING when displayed in the current buffer.
Width is measured by how many columns it occupies on the screen.

[minad]

Yes. One could say that 🤣 But string-width ignores the properties, I tested it. No, seriously - string-width is a pretty low-level function which only cares about composition of characters, it is not about displaying, but about character encoding. Take a look at the c code. I would rather say that the docstring of the function should be fixed.

@hmelman Does it work for you now? I am not using emojis in my setup, I cannot test. Emacs 27 on Linux is not very solid with respect to complicated inventions like emojis.

[minad]

See also my comment here:

consult/consult.el

Lines 607 to 610 in 921e9a5

	;; Avoid allocation for the full string.
	;; There should be a `substring-width' provided by Emacs.
	;; TODO: Propose upstream? Alternatively propose
	;; this whole `display-width' function to upstream.

I think such a display-width function could be useful in upstream (maybe with recursion). And there should be a way to compute the string-width for ranges such that it can be written without allocations.

[oantolin]

The docstring for such a display-width function already exists in Emacs, now we just need the function itself! 😉

[hmelman]

@hmelman Does it work for you now? I am not using emojis in my setup, I cannot test. Emacs 27 on Linux is not very solid with respect to complicated inventions like emojis.

Is all I need to test the current consult--display-width?

[hmelman]

Not quite.

[minad]

Well, this is an Emacs bug 🤷‍♂️ The emoji composition does not work correctly it seems. As an example where it works, if you call (length "(zwsp)") vs (string-width "(zwsp)"), then string-width will correctly return 0, where zwsp should be a zero width space character.

[minad]

Another example that works, depending on perspective 😆

(string-width "⟸") ;; returns 1
(length "​⟸") ;; returns 2

I think emojis are somehow special with how they compose. I have no idea.

[minad]

I am getting more and more unsure about this string-width function. What is it supposed to solve actually? Look at the arrow example above, where the arrow obviously has width 2. Length probably reports that value by accident, but the string-width value is also not correct.

[hmelman]

I get the same results as shown above. FWIW (on the macport):

(string-width "👨‍🍳")  ;; returns 4
(length "​👨‍🍳") ;; returns 4

I cut and pasted the above into base Gnu Emacs -q and while the emoji display as wide blanks (no chef image), the return values are the same, 4 for both.

[minad]

Yes, this means that the composition table used internally by Emacs is broken or outdated. However the frontend gets it right on mac. This is no wonder since I guess the mac frontend uses its own rendering code which does not rely on some outdated Emacs internals.

[hmelman]

So this is what I have now and it seems to be working well. Comments welcome.

(defvar hrm-notes-sources-data
  '(("Restaurants"   ?r "~/Dropbox/Restaurants/")
    ("Lectures"      ?l "~/Dropbox/Lectures/")
    ("Simplenotes"   ?s "~/Library/Application Support/Notational Data/")))

(defun hrm-notes-make-source (name char dir)
  "Return a notes source list suitable for `consult--multi'.
NAME is the source name, CHAR is the narrowing character,
and DIR is the directory to find notes. "
  (let ((idir (propertize (file-name-as-directory dir) 'invisible t)))
    `(:name     ,name
      :narrow   ,char
      :category ,notes-category
      :face     consult-file
      :annotate ,(apply-partially 'hrm-annotate-note name)
      :items    ,(lambda () (mapcar (lambda (f) (concat idir f)) 
				    (directory-files dir nil "[^.].*[.].+")))
      :action   ,(lambda (f) (find-file f) (markdown-mode)))))

(defun hrm-annotate-note (name cand)
  "Annotate file CAND with its source name, size, and modification time."
  (let* ((attrs (file-attributes cand))
	 (fsize (file-size-human-readable (file-attribute-size attrs)))
	 (ftime (format-time-string "%b %d %H:%M" (file-attribute-modification-time attrs))))
    (put-text-property 0 (length name) 'face 'marginalia-type name)
    (put-text-property 0 (length fsize) 'face 'marginalia-size fsize)
    (put-text-property 0 (length ftime) 'face 'marginalia-date ftime)
    (format "%15s  %7s  %10s" name fsize ftime)))

(defun hrm-notes ()
  "Find a file in a notes directory."
  (interactive)
  (consult--multi (mapcar '(lambda (s) (apply 'hrm-notes-make-source s))
			  hrm-notes-sources-data)
		  :prompt "Notes File: "
		  :history 'hrm-notes-history))

I also have similar length of embark integration with 3 simple commands that extend file types. I'll put this in the wiki but how would you like it? Currently the consult wiki has one page with sections that are much shorter. My initial instinct is to make a new page for this, but I'll do whatever you want.

[minad]

Yes, please make it a new page!

[hmelman]

Done. hrm-notes

[minad]

Great, thanks! I will read through this at some point and may give some comments.

[oantolin]

I'm not sure it's worth using a variable to contain the symbol you use for the category. Obviously, there's nothing wrong with that, but it seems unnecessary.

EDIT: This is wrong, I misread the interactive spec on dired-jump:

Also, you could use dired-jump directly instead of the hrm-notes-dired wrapper. You would loose the "Note:" prompt and the note specific docstring, but you would gain the ability to use C-u to indicate you want the dired buffer in another window.

[oantolin]

That reminds me: I wanted to add dired-jump to embark-file-map!

[hmelman]

I'm not sure it's worth using a variable to contain the symbol you use for the category. Obviously, there's nothing wrong with that, but it seems unnecessary.

Yeah I first hardcoded it and when I changed the category I found it easier to make it a variable since it's used in 3 places. If this were a consult-notes package I'd agree, but as a user config, I think it works?

Yeah dired-jump is great but its spec isn't ideal. FWIW I really like the ability to kick off a consult-ripgrep from a file list.

[oantolin]

I added an embark-dired-jump to workaround the dired-jump spec. oantolin/embark@6d68f2f

[hmelman]

Nice. I like my binding of it to d. I'm a little uncomfortable having delete-file be so easy to do. I see there are a few delete commands on d in embark maps and also several find-definition bindings on d. That seems dangerous to me. Have you thought about moving more dangerous commands to harder to type bindings? I see there's a delete-directory binding too and I see that there's some consistency between the maps to maintain.

Also, now that transient will be included in Emacs would it be possible to integrate it into embark? I've become a big fan of their ability to spatially organize the commands in a popup.

[oantolin]

There is already a mechanism in Embark to make it a little harder to delete your files accidentally: in the default configuration you are required to confirm deleting a file by pressing RET ---and of course RET and C-g aren't your only options: you can edit the name of the file to delete. (So in a sense, the keybinding to delete a file isn't d but d RET.) Are you suggesting additionally changing d to a harder to type binding, or are you suggesting the harder binding instead of requiring confirmation with RET?

A transient prompter for Embark would be nice. And probably not too hard to do.

[hmelman]

I hadn't played with it but have now tried it. The confirmation is better though RET is very easy to hit. I'm surprised there isn't a yes/no confirmation, but I see it's a basic emacs function. I don't use it, I typically delete a file via dired which I find easy to do and hard to accidentally do. :)

I'd have to think more about it but my initial thought would be that non-undoable commands should have more difficult bindings. I'd rather have it on D than d (though I don't know where I'd put delete-directory) or maybe C-d. d does make sense and matches dired, so it has that going for it. Though having d be find-definition sometimes I fear would screw up my muscle memory (though maybe the contexts are different enough).

If no one else is complaining then it's probably just me thinking too much about it. Now that I'm done with getting notes working I'll look into embark more. :)

[oantolin]

Maybe find-definition could be moved from d to ., like the M-. binding for xref-find-definitions.

[hmelman]

That was my initial thought too (it's what I would expect on .), though I wanted to look over all the keymaps before suggesting it. But if you like it, go for it.

[oantolin]

Also, I wanted to reuse the built-in commands delete-file and delete-directory to drive home the point that Embark actions are just normal commands, but maybe it would be better to have a single delete action that can delete both files and directories. That would save a keybinding and the combined command could be put on the "hard-to-type" D binding.

It's kind of silly that if you call delete-file on a directory you get an error ("Removing old name: is a directory") and if you call delete-directory on a file you also get an error ("Not a directory").

[minad]

We obviously need embark-rmrf on a key which is easy to reach.

[oantolin]

For convenience we can just run embark-rmrf on a timer, and cancel it if the user does pick a different action.