emacs-citar / citar

Emacs package to quickly find and act on bibliographic references, and edit org, markdown, and latex academic documents.
GNU General Public License v3.0
516 stars 55 forks source link

Make initial value for the different actions customizable #129

Closed publicimageltd closed 3 years ago

publicimageltd commented 3 years ago

While debugging that crm problem, I encountered a more conceptual issue. Vanilla Emacs has no sophisticated completion system, it just offers you all strings which match the inital input from the beginning on (like "^initial input). Therefore, the initial input (like "has:pdf") will not filter the list of available items. I had to delete the initial input in order to access the item. It only works if you have the appropriate completion style installed, in my case, that would be orderless. Assuming that one goal of that package is to provide completion with as little requirements as possible, it makes sense to remove that functional dependency.

So my suggestion is to introduce a variable to make the initial input customizable. Here's a draft:


(defcustom bibtex-actions-initial-inputs 
  '((files . "has:pdf")
    (notes . "has:notes")
 ...)

(defcustom bibtex-actions-filter-with-initial-input t
  "Set this to nil of you use an 'unflexible' completion style"
....)

(defun bibtex-actions--get-initial-input (input-type)
 (when bibtex-actions-filter-with-initial-input
  (assoc input-type bibtex-actions-initial-input))

(defun bibtex-actions-open-notes (keys)
    (interactive (list (bibtex-actions-read :initial (bibtex-actions--get-initial-input 'notes)
                                                              :rebuild-cache current-prefix-arg)))
   (bibtex-completion-edit-notes keys))

There would be further stuff to change, like bibtex-actions--affixation. And if we go that route, :initial could actually be changed to just pass a symbol, that would make the code more elegant.

What do you think?

bdarcus commented 3 years ago

Let me just start high level, and see WYT.

Assuming that one goal of that package is to provide completion with as little requirements as possible, it makes sense to remove that functional dependency.

It's not really a goal to work well with vanilla completion. The UI isn't really designed for that.

Keep in mind early iterations of this work (by @mtreca) started with dependencies on selectrum, consult, marginalia AND embark, with the initial idea this would basically be selectrum-bibtex.

But at Daniel's suggestion, we slowly stripped those away.

I do say this in the README:

For bibtex-actions to work correctly, and as you may have come to expect in helm-bibtex or ivy-bibtex, you need to install and configure either prescient or orderless.

Do you not think that's a reasonable position to take?

Keep in mind, this ecosystem is not static; there's been talk recently of adding orderless to core emacs, and this seems likely to happen.

Now ... aside from that general point, more elegant and flexible code is a good thing. I just don't want to pay for wider support with more complexity.

But your proposed examples do look good! Just need to settle how that fits with my point above.

publicimageltd commented 3 years ago

I see. If it's not the goal to be as independent as possible, it makes sense, of course, to rely on orderless or prescient. I guess what motivated my comment was that I really like the idea of relying solely on completing read, which is also why I like selectrum and friends. It sounds so pure and modular. And it would be cool if orderless would be added to emacs, that would definitely solve that aspect of the issue the most elegant way.

bdarcus commented 3 years ago

I mean, it already is awfully modular; selectrum, vertico, and icomplete-vertical are all viable completion package options, and there may well be others.

And embark is very useful, but purely optional.

But the requirement the initial solution was solving was an important one, and this is simplest solution that worked, and provided user flexibility.

I should probably confirm that initial value solution works with prescient though :-)