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

add bibtex-actions-history #120

Merged
merged 16 commits into from
Apr 30, 2021
Merged

add bibtex-actions-history #120

merged 16 commits into from
Apr 30, 2021

Conversation

bdarcus
Copy link
Contributor

@bdarcus bdarcus commented Apr 29, 2021

This adds history support.

It also may address #119, per suggestion from @oantolin in #119 (comment).

... you can just prepopulate the history with the predefined searches, and people can use a command to search history with completion such as consult-history.

Two questions, @oantolin:

First, to "prepopulate the history", Is it really so simple as the below?

(setq bibtex-actions-history '("analysis" "chem"))

Second, how to activate the history?

If I run consult-history, for example, in any random buffer, I get a "no history configured" for that mode error.

Would this be something where I would just need to turn it on, for say, text-mode?? If yes, how? And in this package, or in a suggested README config?

@oantolin
Copy link
Contributor

First, to "prepopulate the history", Is it really so simple as the below?

(setq bibtex-actions-history '("analysis" "chem"))

Yes, that's all you need to do!

Second, how to activate the history?

If I run consult-history, for example, in any random buffer, I get a "no history configured" for that mode error.

Would this be something where I would just need to turn it on, for say, text-mode?? If yes, how? And in this package, or in a suggested README config?

History is only active at certain times, when you are in the minibuffer or at a shell prompt, a REPL prompt, etc. It's not active in regular text buffers. I'm sure you have used history before; when it is active you can cycle through history items using the up and down arrow keys or M-p and M-n. The consult-history command lets you search through history using completion rather than moving through it one item at a time.

For this package, the history we are talking about would be active in the minibuffer when you run one of these commands that prompts you for a reference.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

For this package, the history we are talking about would be active in the minibuffer when you run one of these commands that prompts you for a reference.

As in, I'm done?

Well, I may want to add a defcustom for predefined search strings?

But otherwise, completing-read, etc will handle the details?

@oantolin
Copy link
Contributor

Yes, I'd say you are done.

(I may have explained more than necessary in my previous comment because I found it very odd that you expected consult-history to do something in a text buffer.)

You may not need a defcustom for the predefined searches, maybe just an example of storing them in the history variable in the README would be enough.

But also, I'm not so sure my suggestion of prepopulating the history is really such a good solution. As you use the commands the history grows and makes it harder to find your predefined searches. Maybe they should be keep separate. As a low-tech solution you could do something like this:

(defcustom bibtex-actions-presets nil
  "List of predefined searches")

(defun bibtex-actions-insert-preset ()
  "Prompt for and insert a predefined search."
  (interactive)
  (when-let ((search (completing-read "Preset: " bibtex-actions-presets)))
    (delete-minibuffer-contents)
    (insert search)))

And just tell users to bind that command to some key in minibuffer-local-map. The usage would be as follows: you run any of the bibtex-actions commands, when you realize you wanted a predefined search you run the above command, then you can select (using completion or the arrow keys or whatever) one of the predefined searches, the command then erases whatever you had already typed at the minibuffer prompt and replaces it with the chosen predefined search.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

I'm not so sure my suggestion of prepopulating the history is really such a good solution. As you use the commands the history grows and makes it harder to find your predefined searches.

Yeah, I was wondering about that :-)

Maybe they should be keep separate. As a low-tech solution you could do something like this:

(defcustom bibtex-actions-presets nil
  "List of predefined searches")

(defun bibtex-actions-insert-preset ()
  "Prompt for and insert a predefined search."
  (interactive)
  (when-let ((search (completing-read "Preset: " bibtex-actions-presets)))
    (delete-minibuffer-contents)
    (insert search)))

And just tell users to bind that command to some key in minibuffer-local-map. The usage would be as follows: you run any of the bibtex-actions commands, when you realize you wanted a predefined search you run the above command, then you can select (using completion or the arrow keys or whatever) one of the predefined searches, the command then erases whatever you had already typed at the minibuffer prompt and replaces it with the chosen predefined search.

That seems better indeed, and low-tech seems appropriate here.

So I could include those two bits of code, and we could add an example of how to configure that on the wiki.

@apc - care to experiment with this idea?

Thanks @oantolin!

@apc
Copy link
Contributor

apc commented Apr 29, 2021

Awesome! Will have a look. May not have time today, but will get back to you tomorrow or so.

@apc
Copy link
Contributor

apc commented Apr 29, 2021

Quick check: is it by design that when navigating through the history I see the entire string with all the spaces in between that, I imagine, are there only for formatting purposes?

image

Also, is it by design that repeatedly pressing M-n takes me through the entire list of entries, even entries that I haven't searched for?

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

Quick check: is it by design that when navigating through the history I see the entire string with all the spaces in between that, I imagine, are there only for formatting purposes?

Is that inserting the candidate string, rather than the search that led to that?

BTW, I added that code to the branch.

Also, is it by design that repeatedly pressing M-n takes me through the entire list of entries, even entries that I haven't searched for?

I'm a little unclear on how you access that history from selectrum, since those kbs by default migrate among the candidates.

@oantolin
Copy link
Contributor

oantolin commented Apr 29, 2021

I'll let @bdarcus say whether it is by design, that is whether it is intended in this case, but I can confirm that is how the normal Emacs history mechanism works.

First, the history items are full selected candidates from previous runs, not just the bit you typed before selecting a candidate (say, with the arrow keys and RET) and are displayed just as they would be when you selected them the first time, containing all the special formatting present.

Second, M-n moves you forward through history, but when it reaches the end of the history it starts moving through "future history" (see the bit about future history in (info "(emacs)Minibuffer History")) . The future history can be manipulated by commands that call completing-read and contains all the candidates by default. A nice use of future history you may be familiar with, is that if there is a file name at point in some buffer you are editing and you call find-file, it puts the file name as the first element of the future history.

@oantolin
Copy link
Contributor

Come to think about it, @bdarcus, maybe the future history is a good place to put the predefined searches. The history grows with use but the future history is whatever the developer puts there.

@oantolin
Copy link
Contributor

Putting the predefined in searches in the future history can be done in addition to any other approach you might also want to implement.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

I'll let @bdarcus say whether it is by design, that is whether it is intended in this case, but I can confirm that is how the normal Emacs history mechanism works.

So for example, if there are multiple candidates that a user selected, this would insert them all in that CRM prompt?

Therefore, it's not the history of what you see in the list of candidates, but of what you select among them?

I didn't realize that.

I think this may just be another place where the default CRM UI in these tools just does not play well with the design of these bibtex-completion frontends, which rely on long candidate strings.

I need to think how well this would work, even with the positive (for these cases) changes contemplated in radian-software/selectrum#489.

This isn't how the predefined search approach in ivy-bibtex and helm-bibtex work, but that's not necessarily a problem. Will just have to think about this a little more.

Thoughts @apc?

Come to think about it, @bdarcus, maybe the future history is a good place to put the predefined searches. The history grows with use but the future history is whatever the developer puts there.

OK, thanks!

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

I need to think how well this would work, even with the positive (for these cases) changes contemplated in raxod502/selectrum#489.

I think if the completion UI (in this case selectrum, but it works similarly in icomplete-vertical and vertico) allowed for a more helm-like selection and indication of candidates, this would be a non-issue.

@apc
Copy link
Contributor

apc commented Apr 29, 2021

I'll let @bdarcus say whether it is by design, that is whether it is intended in this case, but I can confirm that is how the normal Emacs history mechanism works.

So for example, if there are multiple candidates that a user selected, this would insert them all in that CRM prompt?

Therefore, it's not the history of what you see in the list of candidates, but of what you select among them?

I didn't realize that.

I think this may just be another place where the default CRM UI in these tools just does not play well with the design of these bibtex-completion frontends, which rely on long candidate strings.

I need to think how well this would work, even with the positive (for these cases) changes contemplated in raxod502/selectrum#489.

This isn't how the predefined search approach in ivy-bibtex and helm-bibtex work, but that's not necessarily a problem. Will just have to think about this a little more.

Thoughts @apc?

Yes, I think there's something a bit unfortunate about having the entire candidate string populate the prompt either when navigating through history or when selecting multiple entries to pass to one of the bibtex-actions- functions. I read the thread @bdarcus just referred to in the previous comment, and it does look as if the idea of having selected candidates stack at the top (and of showing the history at the top, perhaps) would be a nice solution. Another, perhaps easier-to-implement solution would be to at least remove all the extra spaces that are there for alignment purposes, so you would show a much more compact string when going through the history or when selecting multiple candidates.

Incidentally, I no longer see how to combine the saved queries as just part of the history, since presumably the queries wouldn't correspond to a single candidate. Earlier I thought that the history would be populated by search strings, in which case you would be able to save helpful searches that way. But if it is populated with individual candidates, I'm not sure how this would help e.g. save a search for all entries having some word in the title. @oantolin were you thinking about the saved searches differently than I am?

@oantolin
Copy link
Contributor

The things automatically stored by Emacs in the history are full chosen crm-separated candidate sets. The things manually put in the future history by the developer are arbitrary strings, you can put any sensible query you want there.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

Alright, time to play with future history.

This seems a good post:

https://engineering.collbox.co/post/working-faster-in-emacs-by-reading-the-future/

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

There's discussion of future history, with examples, also on the consult issue tracker:

minad/consult#96

EDIT: in fact, this would seem to be the general solution I was earlier looking for. Using future history could replace the current "initial-value" approach I merged yesterday for the precooked filters.

Consult, IIRC, has an :add-history parameter for these future history data on consult--read. We could add the the same here?

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 29, 2021

Here's a minimal example, @apc, that allows to see how it would work:

(completing-read "Select: " 
  (list "one text" "two text" "three text" "four file") nil nil nil nil '("text" "file"))

The future history list is in the "default" slot.

EDIT: just pushed the start of this change; not tested yet though.

@apc
Copy link
Contributor

apc commented Apr 29, 2021

Neat! Will play around with it next time I have a chance.

@apc
Copy link
Contributor

apc commented Apr 29, 2021

I keep getting this Wrong type argument: char-or-string-p error when calling bibtex-actions-insert-citation and others, whenever I try to navigate through the history:

image

Also, calling bibtex-actions-insert-citation gives me this

image

But I assume you want the value of bibtex-actions-presets that shows there, no?

bibtex-actions.el Outdated Show resolved Hide resolved
@apc
Copy link
Contributor

apc commented Apr 30, 2021

Very nice! One question: do you want the first item in bibtex-actions-presets to show as the 'default' whenever one calls one of the bibtex-actions-open functions? It's not a big deal, but it would be cleaner IMHO if you didn't get saved searches suggesting themselves all the time.

image

(Incidentally: did you consider renaming bibtex-actions-open with bibtex-actions-open-link or some such? It's a bit counterintuitive that the 'open simpliciter' function opens the link, as opposed to the entry, or the PDF. I always need to remind myself that bibtex-actions-open is the one that opens the url...)

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

I sort of prefer the bibtex-actions-insert-preset approach to the future history approach.

To clarify, you mean you prefer the full candidate set to be added, as with your first screenshots in this PR?

That said, it now looks as if bibtex-actions-open is filtering out (by default) all entries that lack either a PDF or a link, but if the intent is to have that function open either PDF or link, this seems like a bug.

Yes. I think it's searching now for presence of both.

I think I need to change that to a regex to get the or.

@oantolin - what's the syntax for has:pdf orhas:link in orderless?

@apc
Copy link
Contributor

apc commented Apr 30, 2021

I sort of prefer the bibtex-actions-insert-preset approach to the future history approach.

To clarify, you mean you prefer the full candidate set to be added, as with your first screenshots in this PR?

No, sorry. At the moment I take it there are two ways of accessing the saved searches: typing M-n from the mini buffer (after e.g. calling bibtex-actions-open) or by calling bibtex-actions-insert-preset (at least after enabling recursive mini buffers, as in the definition in my previous comment), which prompts you then with a list of saved searches you can choose from. I said I preferred the latter to the former. But I take that back. There's something nice about being able to browse through the filtered results as you move from one saved search to another using M-n, something you don't get if you just call bibtex-actions-insert-preset.

@oantolin
Copy link
Contributor

I think you want to add (enable-recursive-minibuffers t) inside the scope of when-let in the definition of bibtex-actions-insert-preset

You are right, @apc, sorry for the oversight. I use (setq enable-recursive-minibuffers t) and forget that not everyone does!

@oantolin - what's the syntax for has:pdf orhas:link in orderless?

orderless only matches against candidates strings. I don't know if you already put the information of whether an item has a link or pdf in your candidate strings, @bdarcus, but if you don't there is nothing orderless can do here.

If you do add the string "has:pdf" (or similar, like "#pdf") to items that have a pdf, then a user could use "has:pdf" to match them (of course that would also match items without a pdf but that happen to have "has:pdf" in the title, or were authored by "steve has:pdf", but one hopes that is unlikely). If an orderless user has a style dispatcher for orderless-without-literal, say like the one in the readme that uses !, they can match references without a pdf using "!has:pdf".

If you do add "has:pdf" or "#pdf" or some sort of marker to the candidates that have pdfs, but don't want this marker visible you can add the invisible property to the marker: even invisible text can be matched against.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

@oantolin -

If you do add "has:pdf" or "#pdf" or some sort of marker to the candidates that have pdfs, but don't want this marker visible you can add the invisible property to the marker: even invisible text can be matched against.

Yeah, I already have this. Sorry; I should have explained more.

The problem @apc noticed is that I add this string to the initial value for the "open" command:

has:pdf has:link

The problem is that will only match candidates that have both, of course, when what we need is to match either/or; the sort of thing you might do with a traditional regexp like:

(has:pdf|has:link)

Can I do that with orderless?

I was trying different variations, but nothing worked.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

I said I preferred the latter to the former. But I take that back.

Ok, so initially you were advocating to keep bibtex-actions-insert-preset, and now are thinking it's fine to ditch it?

I just pushed the change you suggested, and it works.

I'm not sure myself.

I guess we could just include it and see what a wider circle of users think.

EDIT: I did just note one advantage of the "insert" approach is you can include more than one preset in the prompt.

Also, the future history UI works exactly the same in vertico, and (I assume, though haven't tested) icomplete-vertical.

@oantolin
Copy link
Contributor

(has:pdf|has:link)

Can I do that with orderless?

Oh, I see. Well you can't use traditional syntax for regexps in orderless, but you can certainly use Emacs syntax for regexps (of course you need to use regexp matching style for this, but it is on by default). The Emacs syntax uses \|, so you can use has:pdf\|has:link (there's no need for parenthesis here), or has:\(pdf\|link\).

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

The Emacs syntax uses \|, so you can use has:pdf\|has:link

The one variant I hadn't tried!

Awesome; thank you!

Edit: nevermind.

I did just add a note to the README strongly recommending orderless or prescient.

@apc
Copy link
Contributor

apc commented Apr 30, 2021

I said I preferred the latter to the former. But I take that back.

Ok, so initially you were advocating to keep bibtex-actions-insert-preset, and now are thinking it's fine to ditch it?

I think there’s a place for both, especially in light of your point about combining predefined searches below. At first I couldn’t see a reason to have the future history solution other than it made use of stuff that was already there. But after playing around with it I see it’s quite a nice way to implement this feature too.

I just pushed the change you suggested, and it works.

I'm not sure myself.

I guess we could just include it and see what a wider circle of users think.

fair enough!

EDIT: I did just note one advantage of the "insert" approach is you can include more than one preset in the prompt.

Neat!

Also, the future history UI works exactly the same in vertico, and (I assume, though haven't tested) icomplete-vertical.

exactly the same as it does with order less/selectrum or exactly the same as bibtex-actions- insert-preset?

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

exactly the same as it does with order less/selectrum or exactly the same as bibtex-actions- insert-preset?

I mean the UX you get in selectrum is the same with the others, whether using future history, or bibtex-actions-insert-preset.

Not surprising, but helpful to have confirmed.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

Should we update the use-package example on the README to include a bibtex-actions-preset binding?

If yes, any suggestions?

@apc
Copy link
Contributor

apc commented Apr 30, 2021

Should we update the use-package example on the README to include a bibtex-actions-preset binding?

If yes, any suggestions?

Good point. How about this?

(use-package bibtex-actions
  :bind (("C-c b" . bibtex-actions-insert-citation)
         :map minibuffer-local-map
         ("M-b" . bibtex-actions-insert-preset))
  :after embark
  :config
  ;; Make the 'bibtex-actions' bindings available from `embark-act'.
  (add-to-list 'embark-keymap-alist '(bibtex . bibtex-actions-map)))

A mnemonic could be M-b for 'bookmarked`?

Also, if you want to include a bit in the use-package declaration specifying the path to the bib file, you can put this:

(use-package bibtex-actions
  :bind (("C-c b" . bibtex-actions-insert-citation)
         :map minibuffer-local-map
         ("M-b" . bibtex-actions-insert-preset))
  :after embark
  :config
  ;; Make the 'bibtex-actions' bindings available from `embark-act'.
  (add-to-list 'embark-keymap-alist '(bibtex . bibtex-actions-map))
  :custom 
  (bibtex-completion-bibliography '("~/bib/references.bib")))

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

OK, I think this is RTM; your extended use-package example included @apc.

I was thinking I should add you two as co-authors on the commit.

If that sounds good, can you get me your email addresses, or just tell me it's OK to leave it out?

https://docs.github.com/en/github/committing-changes-to-your-project/creating-a-commit-with-multiple-authors#required-co-author-information

@apc
Copy link
Contributor

apc commented Apr 30, 2021

Either way is fine by me! My email is my University's email address, using my GH username before the @ sign.

@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

Either way is fine by me! My email is my University's email address, using my GH username before the @ sign.

Full name or gh user name on the commit, @apc?

@apc
Copy link
Contributor

apc commented Apr 30, 2021

Don't really mind either way. gh user name is shorter! Full name is 'Alejandro Pérez Carballo'.

@bdarcus bdarcus merged commit b1ddbb3 into main Apr 30, 2021
@bdarcus bdarcus deleted the history branch April 30, 2021 15:31
@bdarcus
Copy link
Contributor Author

bdarcus commented Apr 30, 2021

Done. Thanks much to you both.

I fixed the bug with open while I was at it, BTW.

@oantolin
Copy link
Contributor

oantolin commented Apr 30, 2021

One question: do you want the first item in bibtex-actions-presets to show as the 'default' whenever one calls one of the bibtex-actions-open functions?

@apc, this is a Selectrum thing. Other completion UIs don't not aggressively promote the default value like that. In default completion, Icomplete and Vertico the default is not shown unless you include it by hand in the prompt.

@oantolin
Copy link
Contributor

I was thinking I should add you two as co-authors on the commit.

I also don't mind either being included or not; up to you, @bdarcus. My name and email can be found by doing M-x describe-package RET embark RET. 😛

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.

3 participants