minad / vertico

:dizzy: vertico.el - VERTical Interactive COmpletion
GNU General Public License v3.0
1.45k stars 58 forks source link

Run vertico-mode without requiring vertico #219

Closed noctuid closed 2 years ago

noctuid commented 2 years ago

Selectrum puts its define-minor-mode inside a progn, so that the entire definition is included in the generated autoloads (fallback behavior of autoload cookie when form is not recognized as a definition). This allows running (selectrum-mode) without requiring selectrum. It would be nice if vertico did something similar to allow deferred loading by default. Maybe it's not such a big deal since vertico is a small package, but it looks like it would just involve adding a progn and and another autoload cookie above vertico--advice.

minad commented 2 years ago

The autoload approach of selectrum is questionable. As you noted, Vertico is very small, so the problem is nonexistant. If you still want to do the "improved" autoloading you can autoload vertico--advice in your config and replicate the vertico-mode definition in your config.

Alternatively if you want to defer vertico-mode you can at least defer it until first input as doom emacs does it. This way, vertico will not add cost to the initial startup. After that you will interact with the minibuffer anyway.

noctuid commented 2 years ago

I wouldn't call it a bad idea or questionable. It's just a matter of different priorities. Selectrum prioritizes startup speed more than not doing something that is a little unusual. It might betray user expectations for calling vertico-mode to not load the package, so I can understand why you don't want to do this. Given vertico's focus on simplicity, it makes sense to not implement this and just let the user handle deferring vertico if they want to.

Vertico is very small, so the problem is nonexistant

Loading lots of small packages on init quickly adds up, and I don't always use the minibuffer right away. I will continue to defer it the way I have been. I'd rather not replicate the vertico-mode definition in case it changes.

minad commented 2 years ago

I think it is a bad idea given common expectations and the intended design of autoloads. In my setup I don't have any package installed which adds complex definitions to the autoload file. Note that the autoload file is not compiled. I agree that loading many small modes at startup sums up. So one had to weigh everything carefully. Most modes can be deferred slightly, some cannot. Vertico can be deferred to first input which is sufficient (but this is not even necessary). Some modes are worth being loaded that soon. In my setup, Vertico belongs to this exclusive set of packages. I think it is worth it. If not, you can work around it manually as I mentioned. The definition is unlikely to change given it's simplicity.

minad commented 2 years ago

Also in order to buy the startup speed argument one would need real profiling data. How much time does it take to load Vertico in your setup?

hmelman commented 2 years ago

I may be wrong here but I think there are three classes of emacs users relevant to startup speed:

There are also emacs developers who tend to need to restart emacs more often but I don't think this is a lot of users.

minad commented 2 years ago

@hmelman Yes, this is true. I belong to the class of GUI users but I still restart Emacs often to get a clean state. My Emacs starts in about 0.3-0.4s which is okay. I don't use the emacsclient but I defer almost all packages except packages like the themes.

noctuid commented 2 years ago

I wasn't trying to argue with your decision. I think it's the correct one for vertico. I just switched from selectrum due to some weird display issues I was having (and am going to try to do the same for corfu due to glitchy display with company-posframe). vertico just works. This is especially impressive given how small it is. Ivy started off small at ~350 lines but is now over 5000, but vertico won't have that issue. I normally hate the unix/"do one thing well" philosophy, but it works really well inside Emacs where small packages (orderless, consult, vertico, embark, marginalia, etc.) can actually integrate well with each other.

I use emacsclient but also start Emacs a lot for fresh state when testing some configuration or package as well. Even if I only used the daemon, I'd prefer to keep my init time at about half a second. Many years ago (and on a much older laptop), my init time was 20 seconds, which was awful even with the daemon. I now defer everything as much as possible out of principle (even if it doesn't matter much), only requiring packages that are actually used directly in my init.el. I may occasionally open Emacs and use it for a while without using the minibuffer depending on what I am working on, so I will continue to defer vertico in my personal config for consistency.

minad commented 2 years ago

I wasn't trying to argue with your decision.

I didn't mean to imply that you did. Also I wasn't too interested in a long debate, therefore I toned down my first comment from "bad idea" to "questionable" as you noticed. Note that if you come with good arguments it should be possible to convince me. But in this case it is unlikely since there are simply too many reasons (some of which I didn't even mentioned yet) to avoid the method used by Selectrum. Selectrum takes many questionable design choices at other more important places, e.g., its filtering pipeline, which is not compatible with many Emacs completion tables. I started with Selectrum but when I realized this I tried to fix it but then it turned out that it was just easier to package up the proper implementation instead of trying to patch up Selectrum.

I normally hate the unix/"do one thing well" philosophy, but it works really well inside Emacs where small packages (orderless, consult, vertico, embark, marginalia, etc.) can actually integrate well with each other.

I also don't like the unix philosophy, in particular if it means to use numerous command line tools which fit together poorly. But modularity as a concept is not bad. The unix philosphy within Emacs is a good thing. Maybe I am overdoing it a bit with Vertico here, which is highly modularized with all its extensions. But this was my way of showing that something full featured (~ on the level of Helm) is possible, while still retaining a minimal core implementation.

I will continue to defer vertico in my personal config for consistency.

How do you do this? Do you use a delay? I don't like delays since these lead to Emacs being unusable for a certain time. I defer many packages but I always do it in a way which ensures 100% functionality from the start (~0.3-0.4s startup time for me). The only way for Vertico is the following, besides deferring until first input.

(defvar vertico-mode t) ;; maybe not even needed
(autoload 'vertico--advice "vertico")
(advice-add #'completing-read-default :around #'vertico--advice)
(advice-add #'completing-read-multiple :around #'vertico--advice) ;; maybe not needed, rarely used
noctuid commented 2 years ago

Maybe I am overdoing it a bit with Vertico here, which is highly modularized with all its extensions.

I think it's working well here. People can use what they want and leave the rest, and it doesn't require any extra configuration because of the modularization. I'm trying to do this more with my own packages, in part because one giant package also hurts discoverability and can be overwhelming (this has been my experience with general.el - it does way too much for one package).

I don't like delays since these lead to Emacs being unusable for a certain time

I do this for packages I don't need immediately and have no other good way of deferring. I use an equivalent of doom's :defer-incrementally for these so that loading is spaced out during idle time to avoid any freezes. I also use it for packages with a lot of dependencies (like magit), so that the load time when I first use magit-status-here is 0 or less than normal if there has been idle time. Doom has a lot of interesting init.el configuration utilities that are unfortunately unpackaged. I'm currently working on making a package for utilities like this.

How do you do this?

Before I was doing something similar to what doom was by using a helper to add a function pre-command-hook that enabled vertico-mode and then removed itself from the hook. Now I am putting the vertico-mode definition in my config to defer it until it's actually needed since you suggested it won't change.

minad commented 2 years ago

I'm trying to do this more with my own packages, in part because one giant package also hurts discoverability and can be overwhelming (this has been my experience with general.el - it does way too much for one package).

Yes. I can confirm this. In the case of general.el for a long time I didn't know what this package provides. The only package for which I somewhat accept that it is so huge and monolithic is org-mode. But if you look at the innards of org-mode, the code quality is not great and there is a lot of stuff which could be packaged up in a more reusable way such that more packages could profit from it.

I'm currently working on making a package for utilities like this.

Sounds good. I saw your familiar.el. Is that it?

Before I was doing something similar to what doom was by using a helper to add a function pre-command-hook that enabled vertico-mode and then removed itself from the hook.

This is what I am doing - this is what I mean by first input. Doom uses the same terminology I think.

noctuid commented 2 years ago

Yes. I can confirm this. In the case of general.el for a long time I didn't know what this package provides. The only package for which I somewhat accept that it is so huge and monolithic is org-mode.

General.el is mostly related functionality (all of it is for init.el configuration and mostly for keybindings), but a lot of it doesn't need to be in the same repo. It's is not nearly as massive as some packages at only ~2k LoC, but the readme is overwhelming. Bad for discoverability, and a lot of people aren't going to install it just to get one helper function or macro they want. I'm going to split it into 6+ repos, so more people can pick and choose what they actually want.

I think I'm going to try to mimic vertico and just have the main package be a barebones key definer extension system that's only a few hundred LoC. Most keywords (including evil support, which many people never need) will be added in separate files that are not loaded by default. Only the most useful extensions will be included in the same repository.

Sounds good. I saw your familiar.el. Is that it?

That will be one of the new packages. I've decided to call the one I was talking about satch.el (documented but not yet really functional). Still a fairly big readme, but I'm not sure it makes sense to split further (at least not into more repos).

This is what I am doing - this is what I mean by first input. Doom uses the same terminology I think.

I had forgotten that Doom redirects things to new "first" hooks in this way. This is pretty reasonable from the user-perspective but a little strange to me. I've taken a different approach to just use a named variable as a condition to run some code once only.

How are you doing it? Are you using the same code Doom uses? Since you are presumably already using transient hooks/advice, satch.el may not be useful to you, but if you decide to take a look, any feedback on it (or on possibly splitting it further) would be welcome.

(advice-add #'completing-read-default :around #'vertico--advice)

I was also curious about this. Most other similar packages seem to just use completing-read-function. Is it using advice just be more easily undoable, or is there another reason?

(defvar vertico-mode t) ;; maybe not even needed

It looks like vertico-mode is used nowhere currently unless the user checks it in their config.

minad commented 2 years ago

I'm going to split it into 6+ repos, so more people can pick and choose what they actually want. I think I'm going to try to mimic vertico and just have the main package be a barebones key definer extension system that's only a few hundred LoC. Most keywords (including evil support, which many people never need) will be added in separate files that are not loaded by default. Only the most useful extensions will be included in the same repository.

Sounds good. Maybe some of the helpers would even make sense in Emacs itself? Did you consider upstreaming parts of it?

How are you doing it? Are you using the same code Doom uses? Since you are presumably already using transient hooks/advice, satch.el may not be useful to you, but if you decide to take a look, any feedback on it (or on possibly splitting it further) would be welcome.

Yes, the hook I use is mostly like Doom I think, but a bit more stripped down in general. I only define very few macros to get the ball rolling, but otherwise rely only on builtins. I also don't use use-package anymore since it also does a lot of stuff which I didn't need and it wasn't very actively maintained in recent years, which bothered me a little bit when I try to fix some minor details. Maybe I find time to look at your satch.el package, but I don't promise anything. ;)

I was also curious about this. Most other similar packages seem to just use completing-read-function. Is it using advice just be more easily undoable, or is there another reason?

Icomplete uses the same approach. Advices are more easily undoable and I profit from the minibuffer setup of default completion. I haven't found a downside yet. The main Vertico package is very small, so that speaks for itself.

noctuid commented 2 years ago

Sounds good. Maybe some of the helpers would even make sense in Emacs itself? Did you consider upstreaming parts of it?

Possibly, I'm not sure how much of it would make sense to be part of Emacs itself as opposed to just ELPA. Anything tied to evil or another external package wouldn't make sense, though that could be separated. I am still working on getting copyright assignment, but once that's done, it's something I can consider.

I also don't use use-package anymore since it also does a lot of stuff which I didn't need and it wasn't very actively maintained in recent years, which bothered me a little bit when I try to fix some minor details.

I previously saw your issues regarding autoloaded keymaps (e.g. jwiegley/use-package#880). FWIW, I will probably create a standalone package just for use-package's autoloaded keymap functionality.

I like use-package, but I think leaf.el is a little more actively maintained. If you don't need all the functionality, with-eval-after-load and hooks are mostly good enough though.

minad commented 2 years ago

Possibly, I'm not sure how much of it would make sense to be part of Emacs itself as opposed to just ELPA. Anything tied to evil or another external package wouldn't make sense, though that could be separated. I am still working on getting copyright assignment, but once that's done, it's something I can consider.

That would be great! Then we can also easily put back some of the Orderless style dispatchers which we sadly had to kick out.

(I am not using Evil, I think this is one reason why I didn't try general.el. I am not saying that general.el requires Evil, but I've seen or heard that it is recommended in particular if you are evil.)

I previously saw your issues regarding autoloaded keymaps (e.g. jwiegley/use-package#880). FWIW, I will probably create a standalone package just for use-package's autoloaded keymap functionality.

Yes, these were some issues. I don't really have these issues anymore and I think it is also not the best pattern. Another criticism I had regarding use-package is that it sometimes expands to rather large and unoptimized macro definitions. This is of course not really a big deal, but I think it hurts introspection a little bit for beginners. I replaced the meat of it with a handful of very simple macros (+pkg, +bind, +hook, ...). My Vertico config in my init.el looks like this for example:

(+pkg vertico (:first-input vertico-mode) (:set vertico-multiform-categories '((consult-grep buffer)) vertico-multiform-commands '(("find-file" flat (vertico-sort-function . +vertico-directories-first) (+vertico-transform-functions . +vertico-highlight-directory)) ("\consult-imenu" buffer) ("\\consult-buffer" unobtrusive) (consult-outline buffer) (consult-project-buffer unobtrusive) ("\`execute-extended-command" unobtrusive))) (+pkg! vertico-multiform (vertico-multiform-mode) (+bind vertico-map "M-a" vertico-multiform-vertical "M-F" vertico-multiform-flat "M-R" vertico-multiform-reverse "M-G" vertico-multiform-grid "M-U" vertico-multiform-unobtrusive)) (+pkg vertico-directory (:hook rfn-eshadow-update-overlay-hook vertico-directory-tidy) (:bind vertico-map "RET" vertico-directory-enter "DEL" vertico-directory-delete-char "M-DEL" vertico-directory-delete-word)) (+pkg vertico-repeat (:hook minibuffer-setup-hook vertico-repeat-save) (:bind "C-M-," vertico-repeat)))

I like use-package, but I think leaf.el is a little more actively maintained. If you don't need all the functionality, with-eval-after-load and hooks are mostly good enough though.

Yes, exactly. The +pkg block is just with-eval-after-load. There is also setup.el which also seems to be actively maintained.

noctuid commented 2 years ago

That would be great! Then we can also easily put back some of the Orderless style dispatchers which we sadly had to kick out.

Well the legal team agreed that the company has no right to claim my past contributions since they are unrelated to my work, but I still haven't been able to get anyone to sign anything. Hopefully this year though :smile:

it is recommended in particular if you are evil

I wonder what that says about the author.

My Vertico config in my init.el looks like this for example

Thanks for this excerpt. That looks pretty nice. Do you have your init.el anywhere online or could you post the corresponding macros? Are lists starting with keywords just handled specially, and everything else is with-eval-after-load?

I personally prefer to stick with some standard, externally developed package. A minimal "build your own" use-package equivalent might be nice. The expansions of use-package are definitely not as readable as they could be (just tried leaf and it looks better; this is also a major issue with general.el: poor transparency).

There is also setup.el which also seems to be actively maintained.

Thanks for mentioning it. I wasn't aware of it.

I think I like this syntax more (i.e. consistently (:keyword foo) over :keyword foo, :keyword (foo), etc.), and it looks simpler to extend in part because of it. I'm not sure it can handle some of the "x implies :defer t" and autoload generation witchcraft use-package does, but I'm not really sure it was a good idea for use-package to do these things in the first place. I will support it and maybe switch to it.

minad commented 2 years ago

I wonder what that says about the author.

;)

Thanks for this excerpt. That looks pretty nice. Do you have your init.el anywhere online or could you post the corresponding macros? Are lists starting with keywords just handled specially, and everything else is with-eval-after-load?

No, I don't have it published anywhere currently. I may do so in the future. Yes, keywords are handled specifically, e.g., :hook is translated to a call to +hook, but outside the with-eval-after-load block.

I personally prefer to stick with some standard, externally developed package.

I fully agree. Therefore I also recommend use-package throughout all my packages, which is also important for newcomers to get started quickly. My +pkg macro is nothing else than NIH and an experiment. I played around with custom macros when I discussed with a few people who moved away from use-package for various reasons (Prot, David Wilson, ...). But I think my macros are still not as simple as they should be.

A minimal "build your own" use-package equivalent might be nice. The expansions of use-package are definitely not as readable as they could be (just tried leaf and it looks better; this is also a major issue with general.el: poor transparency).

I think setup.el aims to be a lightweight package. But from what I've seen it comes with too much magic for my taste and feels a bit over engineered, despite being relatively light. I think my approach is rather what is the minimal set of macros I can live with instead of how can I put all the features of use-package on a better foundation (setup.el and leaf.el's approach).