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
502 stars 54 forks source link

Add previous inputs to the list of defaults during multiple selection #717

Closed aikrahguzar closed 1 year ago

aikrahguzar commented 1 year ago

Fix #714

This adds the last input to the list of defaults that completing read accepts. As a result it can be quickly inserted using M-n. Indeed all inputs for the current section are available so multiple usage M-n will cycle through them before going through the history.

One consequence of this change is that if one exits with an empty string, the last input gets inserted as one of the references. Previously the first default was set to an empty string to circumvent this. That can be retained but that requires two M-n's or else C-u 2 M-n.

Another possibility is to directly insert the last string using INITIAL-INPUT arg of completing-read or using a hook in minibuffer-with-setup-hook.

bdarcus commented 1 year ago

Thanks!

There's a CI failure that I'm not sure ATM how we'd fix. Any ideas?

aikrahguzar commented 1 year ago

There's an off CI failure that I'm not sure ATM how we'd fix. Any ideas?

I don't even know how the CI works.

bdarcus commented 1 year ago

@roshanshariff there's a compilation error in the CI here related to org. Is there anything we can do to fix it?

bdarcus commented 1 year ago

Notwithstanding the CI, @aikrahguzar, I was just trying this, and not figuring out how it should work.

If I do M-x org-cite-insert and then immediately C-n, vertico just cycles to the first candidate.

Can you clarify?

aikrahguzar commented 1 year ago

Notwithstanding the CI, @aikrahguzar, I was just trying this, and not figuring out how it should work.

Can you clarify?

Type something in the minibuffer while in a multiple selection citar command. Select something using TAB and then press M-n. It should insert whatever you had typed before into the minibuffer.

I should say this is just for the duration of a single call to citar--select-multiple.

bdarcus commented 1 year ago

Select something using TAB and then press M-n. It should insert whatever you had typed before into the minibuffer.

It might be something about how I'm loading it (I just open the file and do emacs-lisp-native-compile-and-load), but that second part doesn't happen ATM for me.

aikrahguzar commented 1 year ago

Select something using TAB and then press M-n. It should insert whatever you had typed before into the minibuffer.

It might be something about how I'm loading it (I just open the file and do emacs-lisp-native-compile-and-load), but that second part doesn't happen ATM for me.

emacs-lisp-native-compile-and-load works for me. What is M-n bound to for you? For me it is next-history-element.

bdarcus commented 1 year ago

User error; was using the wrong keys!

I'll play a bit more now that I have it working.

roshanshariff commented 1 year ago

I've pushed a small commit (#718) that fixes the CI error on Emacs 27. Rebasing this pull request on top of main should fix the failing CI checks here as well.

bdarcus commented 1 year ago

Thanks @roshanshariff!

I just noticed now that this is a bug in org, which Ihor just fixed this morning.

bdarcus commented 1 year ago

One consequence of this change is that if one exits with an empty string, the last input gets inserted as one of the references.

This may be a bit of a problem, as I triggered this without really even knowing how. Playing a bit more, it seems pretty easy to do.

aikrahguzar commented 1 year ago

This may be a bit of a problem, as I triggered this without really even knowing how. Playing a bit more, it seems pretty easy to do.

I think I fixed it just now.

bdarcus commented 1 year ago

I think I fixed it just now.

Initial quick test confirms.

Here's one other little usability thing:

If you recall the previous input string, the cursor is placed at the beginning of it. So you can't just start typing, as you would otherwise; you need a space.

So perhaps just prepend a space to the string?

Also, do you have time to add a brief note about this to the README?

aikrahguzar commented 1 year ago

If you recall the previous input string, the cursor is placed at the beginning of it. So you can't just start typing, as you would otherwise; you need a space.

I don't like the cursor at the beginning but my personal feeling is that the cursor position belongs to completing-read or else the completion UI e.g. vertico. Prepending a space will work for orderless, is not needed for a fuzzy completion style and will not work but styles like prefix.

Also, do you have time to add a brief note about this to the README?

Hopefully tomorrow.

bdarcus commented 1 year ago

Prepending a space will work for orderless, is not needed for a fuzzy completion style and will not work but styles like prefix.

Is it feasible, if not now, later, to have a config variable that would address this?

aikrahguzar commented 1 year ago

Is it feasible, if not now, later, to have a config variable that would address this?

The reason I think if it best not to do this is that we are using a standard emacs mechanism i.e M-n behaves this way for all uses in minibuffer and if a user wants to tweak this behavior they probably want to tweak it everywhere and not just when recalling a previous input in citar.

I looked around why this was happening and it is basically due to this part in goto-history-element,

(unless (memq last-command '(next-history-element
                 previous-history-element))
      (let ((prompt-end (minibuffer-prompt-end)))
        (setq-local minibuffer-temporary-goal-position
                    (cond ((<= (point) prompt-end) prompt-end)
                          ((eobp) nil)
                          (t (point))))))

(bunch_of_code_to_acutally_insert_the_element)

(goto-char (or minibuffer-temporary-goal-position (point-max)))

Because of the first clause in cond, if the input is empty and one presses M-n the cursor is at the beginning (which is unexpected for me) but if one does SPC M-n the cursor is at the end (expected). This seems like a bug and I can report it.

To get around it, the best way seems to be an advice on goto-history-element i.e.

(advice-add 'goto-history-element :after #'end-of-buffer)

And we can mention that in the README.

bdarcus commented 1 year ago

Thanks for digging into that.

This seems like a bug and I can report it.

Yes please.

aikrahguzar commented 1 year ago

Yes please.

Done https://debbugs.gnu.org/cgi/bugreport.cgi?bug=60468

aikrahguzar commented 1 year ago

Hopefully tomorrow.

Now done.

bdarcus commented 1 year ago

Merged; thanks!

And Happy New Year!