-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
embark-act not presenting command/kb options #176
Comments
embark-act
not presenting command/kb options
I'll look into it, but right now I am not able to run the code. Here are a few problems I found:
Are you able to run your code? If so, can you give me some instructions on how to do that? Here's what I tried:
In the meantime I can try the obvious fixes for the problems I found, but would appreciate some help in getting the code to run so I can test. |
Oops, sorry for the noise, I realized you just have a tiny typo: (set 'bibtex-completion-read-backend
(symbol-function 'bibtex-completion--completing-read)) That should use |
Thanks; will fix that.
I will get back to you with the other details later today.
|
One piece of advice that I have independent of Embark is to not reload the bibtex file after every single keystroke! In the |
Will fix that too.
Do you have it running now, or do you example data and config settings?
…On Mon, Mar 1, 2021 at 11:30 AM Omar Antolín Camarena < ***@***.***> wrote:
One piece of advice that I have independent of Embark is to not reload the
bibtex file after every single keystroke!
In the bibtex-completion--completing-read function you should call
(bibtex-completion--get-candidates) *once`, storing the result in a
variable and use that variable inside your completion table function.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#176 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAI3R7PXY6YH3DEPQZ6GLTBO6KBANCNFSM4YMXTTOA>
.
|
I can run it now, no worries. |
Ah, you have the syntax for |
Ugh, sorry for the noise then.
I will fix and report back later. Thank you.
|
After fixing the keymap definition I do get offered the actions in Embark! They don't quite seem to work since they receive the entire formatted reference string shown in the minibuffer and seem to expect receiving only the key. (I don't have PDFs for my references, so I only tried the insert-key, insert-bibtex and show-entry actions.) |
Weird indeed. Particularly because earlier this worked fine. |
Or maybe I misunderstood what you meant by "they." I was assuming you meant
the keymap or something.
I'll take a closer look in a bit.
…On Mon, Mar 1, 2021, 11:42 AM Omar Antolín Camarena < ***@***.***> wrote:
After fixing the keymap definition I do get offered the actions in Embark!
They don't quite seem to work since they receive the entire formatted
reference string shown in the minibuffer and seem to expect receiving only
the key. (I don't have PDFs for my references, so I only tried the
insert-key, insert-bibtex and show-entry actions.)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#176 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAAI3XPNS6ZDE6K5MFFRCDTBO7XLANCNFSM4YMXTTOA>
.
|
I believe your current version is still defining |
With this version of the function everything seem to work here: (defun bibtex-completion--completing-read ()
"Read bibtex-completion entries for completion using completing-read."
(bibtex-completion-init)
(when-let ((candidates (bibtex-completion--get-candidates))
(chosen
(completing-read
"BibTeX entries: "
(lambda (string predicate action)
(if (eq action 'metadata)
'(metadata
;; (annotation-function . bibtex-completion--annotation)
(category . bibtex))
(complete-with-action action candidates string predicate))))))
(cdr (assoc chosen candidates)))) As I said above the action commands were not getting called with the bibtex key, but rather with the string containing all the info that is used for matching. What happens is that Hope this helps! |
Absolutely; thank you very much!
This raises a question: is it feasible and convenient for embark to issue a
warning if the keymap is wrong?
|
It is definitely possible to warn if a variable listed in |
OK :-) Just to confirm everything now works as expected. Thanks again! |
If you have a few moments at some point, @oantolin, conceptually, how should I adapt your modified function (pasted below for convenience) for (defun bibtex-completion--completing-read ()
"Read bibtex-completion entries for completion using completing-read."
(bibtex-completion-init)
(when-let ((candidates (bibtex-completion--get-candidates))
(chosen
(completing-read
"BibTeX entries: "
(lambda (string predicate action)
(if (eq action 'metadata)
'(metadata
;; (annotation-function . bibtex-completion--annotation)
(category . bibtex))
(complete-with-action action candidates string predicate))))))
(cdr (assoc chosen candidates)))) |
Embark doesn't have support for passing lists to actions. The actions receive a single string, the top completion of the minibuffer contents. If you are using a completing-read-multiple UI where the minibuffer has a comma-separated list of candidates (such as the default completion UI, Icomplete, or current Selectrum before any changes in radian-software/selectrum#489 that might affect the minibuffer contents), that entire string with commas and candidates will be passed to the actions. It might be worth changing the crm-separator, if the bibtex candidates can contain commas. EDIT: I said the actions recieve a single string with candidates and commas. The situation is a little better than that. That string is what is injected at the minibuffer prompt of the action, but if the action reads its arguments with |
As for that function that you pasted, I think the only change it needs is to replace |
Thanks for the quick response. First, I'm glad I was correct that the function should work with modifying anything else. On this:
So what does this mean practically for me? The backend bibtex-completion functions do take a list of key strings: ELISP> (bibtex-completion-insert-key (list "zukin_new_2009" "couper_2014"))
nil
ELISP> (bibtex-completion-insert-key "zukin_new_2009,couper_2014")
*** Eval error *** Wrong type argument: sequencep, 122 In the earlier version of the code, it was just always a list of one, I guess. |
So I think that as long as the completion UI you are using was the property that the minibuffer contents represents the entire multiple selection that the only changes required are to the function (defun bibtex-completion--completing-read ()
"Read bibtex-completion entries for completion using completing-read."
(bibtex-completion-init)
(when-let ((candidates (bibtex-completion--get-candidates))
(chosen
(completing-read-multiple
"BibTeX entries: "
(lambda (string predicate action)
(if (eq action 'metadata)
'(metadata
;; (annotation-function . bibtex-completion--annotation)
(category . bibtex))
(complete-with-action action candidates string predicate))))))
(mapcar (lambda (choice) (cdr (assoc choice candidates))) chosen))) Then all these commands that recieve an argument that is a list of bibtex keys interactively using If you want further detail, here's how it will work under the hood: if you run one of those commands and select various bibliography items, the minibuffer will contain a It might be nicer if these selection list weren't serialized to |
It seems selectrum has that characteristic now, and is likely to retain that. Right?
Hah; I was wondering about that!
OK, this is super helpful. I'll see if I can get this working now; first just the basic commands, and then with embark. I actually moved what I was working on to a separate repo because easier for me to manage (branching, issue tracker, etc.). https://github.com/bdarcus/bibtex-actions
This process you describe here is what embark is doing; correct? |
It does have that characteristic now. I didn't follow the full discussion, so I'm not completely sure about future plans. As far I did read, I think the plan was to keep the minibuffer contents the same.
Yeah, sorry. I'm glad you caught it.
If I understand the structure of these functions correctly, they will automatically work with Embark.
Correct. |
OK, great. I integrated the code here ... ... and I'm getting closer to it working. But doesn't appear to find the key, as the output is comma-separated empty spaces. I've been grading all day though, so brain is fried. I'll look again in the morning. Thanks again Omar; really appreciate your patience. |
The bibtex-completion functions all take multiple keys as input, and some of them (notably the "input" ones) lose functionality without support for this. Hence, this changes the core read function to use 'completing-read-multiple'. In order to do that, it also sets the 'crm-separator' to ampersand, since default comma won't work for these data. Many thanks to @oantolin for help on the read function. Addresses #17. cc oantolin/embark#176 (comment)
Not urgent at all @oantolin, but can I ask a followup on this? Your suggestion was this: (defun bibtex-completion--completing-read ()
"Read bibtex-completion entries for completion using completing-read."
(bibtex-completion-init)
(when-let ((candidates (bibtex-completion--get-candidates)) So the question is simple: how long is the Would it be refreshed every time the Trying to sort out how to get file-notify to update the candidates when certain files or directories change. |
Yes. I would leave it at that and not bother with |
Gotcha.
Thank you!
|
Of course, a few hours after this, I got a feature request for caching
from a user with large bib files.
emacs-citar/citar#68
So I guess I'll have to figure this out.
|
I think I'd try caching with a simple manual refresh first to see if that's comfortable. A user can just go about their bibtex business and when they try to insert a reference that they are sure they have but bibtex-actions can't find, they refresh the cache. People have different habits, but I think that for me that would be sufficient. I tend to only add references to my bibtex files in batches, I'd just have to refresh the cache after every batch. |
Makes sense. I actually learned today in subsequent back-and-forth that I already am indirectly caching, because bibtex-completion already has that functionality. But there appears to be a slight delay when I call my commands, probably because the candidate function (which ultimately does call the cached data) is a bit slow.
Me too. The use case I was thinking of is note-taking on sources, say with |
Done: emacs-citar/citar@edb568a Initially, I just added a section to the README with how to manually setup file-notify for this. I may also add a couple of convenience commands that the user can also run manually. Thanks! |
Very cool! One small suggestion I'd make is to have |
If you have an additional moment, can you explain that? Why does that optional argument obviate the lambda. FWIW, as I was working on this, I was wondering why the lambda! |
The |
Thank you; now I understand! Here's the commit: emacs-citar/citar@d31bfa6. |
I am working on this PR, that I am 99% sure should work, but the commands aren't available from
embark-act
. I don't even know where to begin to figure out why.I do know the
embark-keymap-alist
variable is correctly set; here's what's reported bydescribe-variable
.Value: ((bibtex . bibtex-completion-map)
I'm using Emacs 28.0.50.
Any tips?
The text was updated successfully, but these errors were encountered: