-
-
Notifications
You must be signed in to change notification settings - Fork 108
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
Questions about a consult-notes command attempt #210
Comments
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 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. |
Annotations work for me: (nconc consult--source-buffer `(:annotate ,(lambda (cand) (format "==%S==" cand)))) |
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! |
Right, I missed this question:
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. |
I'd suggest just changing
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
|
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.
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? |
@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. |
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. |
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. |
@hmelman Do you see the Marginalia annotations instead? Please disable marginalia-mode and recheck. |
@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. |
Yes, disabling |
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. |
And it looks like with marginalia-mode disabled, it automatically shows the source |
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? |
This is a consult feature. consult--multi-annotate adds the source names.
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. |
More a marginalia question but I'll continue it here. I've changed my sources to use
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. |
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 But maybe setting the default directory in the minibuffer doesn't work, since Marginalia actually uses the value of |
@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. |
@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. |
@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. ;) |
Oh! Sorry @hmelman, I forgot you were trying to combine multiple sources. I think I'd recommend not using the
|
* The annotator should delegate to the original annotation function for categories which are not overriden by Marginalia * Introduce marginalia--metadata variable (replace marginalia--original-category) * Store the completion metadata in marginalia--metadata when annotators and classifiers are executed * See minad/consult#210 for the discussion * Thank you @hmelman for finding the issue!
@oantolin This sounds like what @hmelman is planning to do! |
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? |
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 |
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. |
@oantolin See above, I added a small optimization which avoids allocations for candidates without invisible and display parts, which should be most candidates. |
Have you read the docstring of
|
Yes. One could say that 🤣 But @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. |
See also my comment here: Lines 607 to 610 in 921e9a5
I think such a |
The docstring for such a |
Is all I need to test the current |
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. |
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. |
I am getting more and more unsure about this |
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. |
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. |
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. |
Yes, please make it a new page! |
Done. hrm-notes |
Great, thanks! I will read through this at some point and may give some comments. |
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
|
That reminds me: I wanted to add dired-jump to embark-file-map! |
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 |
I added an |
Nice. I like my binding of it to 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. |
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 A transient prompter for Embark would be nice. And probably not too hard to do. |
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. :) |
Maybe |
That was my initial thought too (it's what I would expect on |
Also, I wanted to reuse the built-in commands It's kind of silly that if you call |
We obviously need |
For convenience we can just run embark-rmrf on a timer, and cancel it if the user does pick a different action. |
I'm trying to use
consult--multi
to manage several directories of notes files. I have the following: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 theconsult--multi
call with a function that returned("Restaurants")
but it didn't show. For file attributes I'll have another problem as thedefault-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.The text was updated successfully, but these errors were encountered: