WIP: Minibuffer overhaul

[Ambrevar]

This is an attempt to address #777 and most (all?) minibuffer-related issues.

First step is to implement the "backend": anything that's not display related, roughly reimplementing Emacs Helm.

Second step is to integrate it in Nyxt by implementing the frontend.

[Ambrevar]

This is based on the Calispel patchset (#1047) because I need it here.
Minibuffer-related changes start November 27.

See the tests for a nice use of Calispel with fair-alt to track suggestion computation progress.

[Ambrevar]

For now there is nothing that can be used in Nyxt, however the backend data structures are ready for review.

Next steps:

• Backport test-fuzzy.lisp to make sure we have at least the same matching quality as master.
• Implement a new Nyxt command to use this new minibuffer, e.g. set-url2.

[Ambrevar]

Any early code review is already welcome! In particular, I'm happy to hear feedback about the class and slot names.

[jmercouris]

why do we need a make-source? what are the benefits over make-instance 'minibuffer-source and then having initialize-instance after?

[Ambrevar]

initialize-instance can be used regardless.
make-source is a tiny shorthand for make-instance 'minibuffer-source. I figured that the user is going to use it a lot, so let's make this as short as possible.

[jmercouris]

OK, that is a good point. I assume you also want to have the key arguments for the user hence why we have to read the slots?

[Ambrevar]

Yes, that's the point of the define-function macro, it allows us to automatically generate the keyword arguments.

[jmercouris]

would just =action= be OK too? I think that would make more sense personally

[Ambrevar]

But having both actions and action is confusing.
Otherwise I can change actions to be a structure of

• list of actions
• default action
• persistent action

[jmercouris]

I had already assumed that actions was a list of action objects, what else would it be?

[Ambrevar]

It is, but it's a bit limited because a simple list of actions does not specify:

• the default action (when the user presses return)
• the persistent action (when follow-mode is on, or when the user presses a dedicated key).

So either we add slots to specify these two, or we set actions to be of the action-list type which has the aforementioned 3 slots.

[jmercouris]

or we could use a plist?

[jmercouris]

OR actions is actually an object and not a list, with methods to return the default action, the persistent action, etc.

[Ambrevar]

A plist could do, but the benefit of a dedicated object is that it catches error (plist accept arbitrary keys) which is very useful to catch typos in the user config.

About methods: the drawback is that it forces the user to define the action list globally to be able to decide which action is the default, which is much more verbose. Compare this (untested) config sample:

;; Without methods
(minibuffer:make
  :sources (list (minibuffer:make-source
                  :actions (minibuffer:make-actions
                            :default default-action
                            :persistent persistent-action
                            :others (list action-a action-b)))))

;; With methods
(defvar *my-actions* (minibuffer:make-actions default-action
                                              persistent-action
                                              action-a action-b))

(defmethod default-action ((actions (eql *my-actions*)))
  (first (minibuffer:actions actions)))
(defmethod persistent-action ((actions (eql *my-actions*)))
  (second (minibuffer:actions actions)))

(minibuffer:make
 :sources (list minibuffer:make-source
                :actions *my-actions*))

[jmercouris]

We can just check if they are or not, no? I guess I don't understand the problem indicated by the TODO here :-D

[Ambrevar]

The problem is that we change initarg data at initialization time, which may be surprising to the user. This is not very good practice in terms of data consistency.

An alternative would be to have a separate slot, e.g. initial-data, and leave initial-suggestions unexported.

Another solution is to remove the initarg for initial-suggestions and add a new keyword argument initial-data to make-source.

[jmercouris]

I would prefer readyp

[Ambrevar]

Forgot to update this one. I prefer ready-p over readyp though.

[jmercouris]

Thats fine, the convention is a bit silly to me that single words don't get a dash and multiple words get a dash. I think it should just always be a dash.

[jmercouris]

Just some surface level comments for now, I haven't dug into the code yet :-)

[Ambrevar]

I'm thinking of renaming initializer and cleanup to constructor and
destructor respectively. This would be consistent with modes.

While we are at it, we can make methods for them as suggested in #1005.
We could still keep the slots which would allow for instance-specific overrides.

[jmercouris]

I agree with that change

[Ambrevar]

Note: make-source should return a lambda that creates minibuffer-source
objects. Indeed, we don't want global, non-thread-safe minibuffer-source
object.
The source object should be instantiated with the minibuffer.

[Ambrevar]

On second though, do we really need a macro to create top-level sources?
What about just using define-class? For instance

(define-class minibuffer-foo-source (minibuffer-source)
  ((initial-suggestions '("foo" "bar"))
   (filter #'slow-identity-match)))

(minibuffer:make
 :sources (list (make-instance 'minibuffer-foo-source)))

Thought?

[jmercouris]

define-class may suffice for the purpose, if you find yourself limited by it, we can always create a macro :-)

[Ambrevar]

Acknowledged. I tried getting input methods to work in the minibuffer. The easiest way to test is with unicode input, e.g. press "control-shift-u 1 2 3 4 space" and see if it inserts a unicode character. It does not work because in our gtk-entry workaround because we only have gtk:gtk-entry-im-context-filter-keypress and no way to tell that some input preedit is happening. One way around it would be to define a custom GTK input method. This seems difficult to get right and it's unclear to me whether it will work well with the various user settings (UIM, IBUS, etc.). https://developer.gnome.org/gtk3/stable/GtkIMContext.html So for now I'm leaning towards using a GtkTextInput widget instead of our home-rolled HTML input. The main reason we had this HTML input initially was because we used Javascript to control the text editing features. But now that we have input-edit.lisp, we can perfectly edit any GTK text input. Another thing to keep in mind is that GTK is capable of applying CSS anywhere, so styling would not be an issue. Thoughts?

[jmercouris]

I am not a huge fan of it because one part of the minibuffer would be native widget, while the other would be a GTK widget. Also, the HTML affords us many benefits that a normal GtkTextInput widget does not. For example, we could implement Hydra really easily within the minibuffer right now. If we use a GtkTextInput, then we won't be able to. What if we had an invisible GtkTextInput that is focused and recieves input, and we just mirror the contents of that into our HTML view?

[Ambrevar]

Can you explain why Hydra would be a problem with GtkTextInput?

What if we had an invisible GtkTextInput that is focused and recieves input, and we just mirror the contents of that into our HTML view?

I'll try that. Another benefit of using a native text widget is that it gives us regions for free.

[jmercouris]

A hydra would be a problem because:

1. it would be renderer dependent implementation
2. It would take up unresolvable space in the minibuffer

[jmercouris]

Also, regions not existing is a limitation of our text-buffer library, we will have them eventually :-)!

[Ambrevar]

1. it would be renderer dependent implementation 2. It would take up unresolvable space in the minibuffer

We can render the Hydra as part of the minibuffer suggestions, so this won't be a problem.

[Ambrevar]

See commit d5204f0. I tried what you suggested, it was a good idea but sdaly it does not seem to work :/ I think it's because intercepting key events breaks the IM context. Can you think of a way around this?

[jmercouris]

If intercepting key events breaks the IM context, then it won't work no matter what. How does Emacs allow insertion of special symbols? We don't necessarily have to use the input manager, is there another way?

[jmercouris]

Could we perahps toggle/untoggle intercepting the IM context? Have a sort of "raw input" button?

[Ambrevar]

Many updates on this branch, it's starting to be quite usable!
Feedback welcome!

You can try out 2 commands that use the new prompt buffer:

• set-url2
• switch-buffer2

Highlights:

• Paging is supported (can be improved in the future).

  • M-< and M-> go to first, resp. last suggestion.
  • C-v and M-v go one page up, resp. down.

• Multiple column support. It's fully customizable via the object-properties
  method and the suggestion-property-function slot.

• Multiple source support. See set-url2.

• Multiple action support. See set-url, press M-return and stand in awe :)

• Persistent action support. See switch-buffer2 and navigate the suggestions:
  the current buffer is automatically switched to the suggestion. On escape,
  the original buffer is restored. C-c C-f toggles "follow mode".

The prompter-library is almost finished.

The prompt-buffer.lisp file is the main spot that needs refactoring. Will do
when I'm done with the rest.

What remains to be done:

• Have prompt-buffer inherit from prompter instead of having a
  prompter slot. This would make the Nyxt side of things way less verbose, it
  will also be easier for the end user to invoke the prompt buffer.
• Rename the prompter:prompter-source class to prompter:source.
• Contextual help display.
• Resume previous prompt-buffers.
• Highlight portion of matching text.
• VI bindings
• Implement search-buffer2 command using prompt-buffer.
• Restore multi-buffer search leveraging the new multiple source feature.
• Make meta command which allows for selecting sources, then drop into new
  prompt buffer with given sources.

The good news is that most of the above tasks are really easy to implement. The
only one I'm unsure about is the search-buffer command. Will see.

Then after a final review, I'll replace all the minibuffer instances with
prompt-buffers.

There are also some cosmetic tweaks I would need help with (pinging @jmercouris :p):

• Auto-focus on HTML input. Maybe focusing on the GTK widget would fix it.
• Display HTML input and prompt on the same line.
• Fit columns to width, and keep their width constant when navigating the
  suggestions.
• While paging works, going "up" (e.g. with C-p) does not recenter
  suggestions that are "under" the input line and the prompt.

Stay tuned!

[Ambrevar]

To do:

• The prompt buffer key bindings should be discoverable (e.g. with C-h b).

[Ambrevar]

Closing in favour of #1157.