add selectrum module #4519
add selectrum module #4519
Conversation
hlissner
added
is:feature
There was a problem hiding this comment.
We'll need selectrum-powered-alternatives for +ivy/project-search, +ivy/project-search-from-cwd, and a project-wide search+replace workflow to replace SPC s p + C-c C-e.
Reaching parity with our ivy module is important, so it's unlikely that this PR (or any selectrum PR) will be merged soon, but it will be a helpful reference when I get around to it.
| (package! selectrum) | ||
| (when (featurep! +prescient) | ||
| (package! selectrum-prescient)) | ||
|
|
||
| (package! consult) | ||
| (package! consult-flycheck) | ||
| (package! embark) | ||
| (package! embark-consult) | ||
| (package! marginalia) |
|
|
|
I've added a function for selectrum project search in this PR. |
|
What does your custom function for project search do that |
It's doom-specific stuff:
These are mainly so that the signature of the functions align with that of ivy and helm, I suppose. |
It seems like |
That's exactly how it's implemented now. |
|
Totally my bad, I looked first on my phone and clearly didn't read the code closely enough. |
| [remap load-theme] #'consult-theme | ||
| [remap recentf-open-files] #'consult-recent-file) | ||
| :config | ||
| (setq consult-project-root-function #'projectile-project-root)) |
There was a problem hiding this comment.
why not use doom-project-root here?
| "\\\\\\\\|") | ||
| ((concat "\\\\" substr)))) | ||
| (rxt-quote-pcre (doom-thing-at-point-or-region)))))) | ||
| (ripgrep-command `("rg" ,@args "." "-e"))) |
There was a problem hiding this comment.
If I'm reading this correctly, you converted args into a string to concatenate more arguments to it inside the let binding, than here, you expand it to a list again in order to store it on ripgrep-command variable.
Wouldn't be more efficient to only do list operations here?
I was thinking on something like this:
(let* ((this-command 'consult-ripgrep)
(project-root (or (doom-project-root) default-directory))
(directory (or in project-root))
(prompt (or prompt
(format "%s [%s]: "
(mapconcat #'identity command " ")
(cond ((equal directory default-directory) "./")
((equal directory project-root) (projectile-project-name))
((file-relative-name directory project-root))))))
(query (or query
(when (doom-region-active-p)
(replace-regexp-in-string
"[! |]" (lambda (substr)
(cond ((string= substr " ") " ")
((string= substr "|") "\\\\\\\\|")
((concat "\\\\" substr))))
(rxt-quote-pcre (doom-thing-at-point-or-region))))))
(ripgrep-command (seq-remove 'null
(append (butlast consult-ripgrep-command)
(list
(when all-files "-uu")
(unless recursive " --maxdepth 1")
"--hidden"
"-g!.git")
args
'("-e")))))
...
)| ((concat "\\\\" substr)))) | ||
| (rxt-quote-pcre (doom-thing-at-point-or-region)))))) | ||
| (ripgrep-command `("rg" ,@args "." "-e"))) | ||
| (consult--grep prompt ripgrep-command directory query))) |
There was a problem hiding this comment.
The ivy counterpart of this function, sets the variable deactivate-mark before running the grep command. Shouldn't you do the same?
| @@ -0,0 +1,64 @@ | |||
| ;;; completion/selectrum/config.el -*- lexical-binding: t; -*- | |||
|
|
|||
| (use-package! selectrum | |||
There was a problem hiding this comment.
Shouldn't you manually set the refine, and highlight candidates functions if +orderless is enabled? Also, if that is the case, and the user has +prescient enabled as well, you should set the preprocesscandidates function as well:
(setq selectrum-refine-candidates-function #'orderless-filter
selectrum-highlight-candidates-function #'orderless-highlight-matches
selectrum-preprocess-candidates-function #'selectrum-prescient--preprocess)There was a problem hiding this comment.
Forgot to mention that if you want to use orderless and prescient, you should tell prescient to remember candidate selection:
(add-hook 'selectrum-candidate-selected-hook #'selectrum-prescient--remember)
(add-hook 'selectrum-candidate-inserted-hook #'selectrum-prescient--remember)
There was a problem hiding this comment.
Setting separate refine and highlight functions is not necessary since Selectrum picks up the completion style, see radian-software/selectrum#330. However you should still set them in order to improve performance, since then Selectrum is applying highlighting only to the items which match the filter criterion! Note that the combination of Orderless and Prescient is a bit fragile since both packages compete about the filtering. I haven't checked what you are doing here, but selectrum-prescient-mode should be activated first, then orderless should overwrite the filter/highlighting functions. Furthermore orderless should be set as completion style.
| (when (featurep! +prescient) | ||
| (use-package! selectrum-prescient | ||
| :after selectrum | ||
| :hook ((selectrum-mode . selectrum-prescient-mode) |
There was a problem hiding this comment.
You should only do this if use does not have +orderless enabled.
There was a problem hiding this comment.
See my comment above, about the order of selectrum-prescient-mode and orderless.
There was a problem hiding this comment.
If you don't enable selectrum-prescient-mode, you can use prescient sorting plus storing with orderless filter and highlights with no problem. If the user don't want this combination, then you don't need to worry as I comment, but overall, my suggestion is to get this decision path:
(cond ((and (featurep! +prescient) (featurep! +orderless)) (set-functions-manually))
((featurep! +prescient) (enable-selectrum-prescient-mode))
((featurep! +orderless) (only-enable-orderless)))So, the use-package! for selectrum would be something like this:
(use-package! selectrum
:init
(when (and (featurep! +prescient) (featurep! +orderless))
(setq selectrum-refine-candidates-function #'orderless-filter
selectrum-highlight-candidates-function #'orderless-highlight-matches
selectrum-preprocess-candidates-function #'selectrum-prescient--preprocess)
(add-hook 'selectrum-candidate-selected-hook #'selectrum-prescient--remember)
(add-hook 'selectrum-candidate-inserted-hook #'selectrum-prescient--remember))
:config
;; other configurations...
)And selectrum-prescient would be:
(use-package! selectrum-prescient
:if (featurep! +prescient)
:after selectrum
: init
(when (not (featurep! +orderless))
(add-hook 'selectrum-mode-hook 'selectrum-prescient-mode))
:config
;; other configurations
)| @@ -47,6 +48,7 @@ If prefix ARG is set, include ignored/hidden files." | |||
| (call-interactively | |||
| (cond ((featurep! :completion ivy) #'+ivy/project-search) | |||
| ((featurep! :completion helm) #'+helm/project-search) | |||
| ((featurep! :completion selectrum) #'+selectrum/project-search) | |||
| (#'projectile-ripgrep))))) | |||
|
|
|||
There was a problem hiding this comment.
You have to change the +default/yank-pop as well:
;;;###autoload
(defun +default/yank-pop (&rest _)
"Interactively select what text to insert from the kill ring."
(interactive "P")
(call-interactively
(cond ((fboundp 'counsel-yank-pop) #'counsel-yank-pop)
((fboundp 'consult-yank-pop) #'consult-yank-pop)
((fboundp 'helm-show-kill-ring) #'helm-show-kill-ring)
((error "No kill-ring search backend available. Enable ivy or helm!")))))| (define-key! | ||
| "C-S-a" #'embark-act) | ||
| ;; TODO need to figure out what keybindings to put here | ||
| (define-key! selectrum-minibuffer-map |
There was a problem hiding this comment.
why are you adding these to selectrum-minibuffer-map? Embark is supposed to work without selectrum, so you can add these directly to the minibuffer-local-map, don't you?
| "C-c C-c" #'embark-act-noexit)) | ||
|
|
||
| (use-package! marginalia | ||
| :after selectrum |
There was a problem hiding this comment.
I'm not sure you need to wait for selectrum to load here. I think just the hook is enough.
| :init | ||
| (define-key! | ||
| "C-S-a" #'embark-act) | ||
| ;; TODO need to figure out what keybindings to put here |
There was a problem hiding this comment.
Embark wiki has excellent suggestions. I particularly like the "Very large files and sudo edit".
| :hook ((selectrum-mode . selectrum-prescient-mode) | ||
| (selectrum-mode . prescient-persist-mode)))) | ||
|
|
||
| (use-package! orderless |
There was a problem hiding this comment.
Can't we lazy initialize orderless as well? If we can't, a comment explaining why would be helpful.
There was a problem hiding this comment.
It should be installed as completion-style, I recommed against loading it lazily. But I am not familiar with the doom infrastructure, maybe lazy loading after the first input would work? It could also be that it would not work since then the completion style is made available a little bit too late.
There was a problem hiding this comment.
Just for context, this is the use-package! orderless section on my config:
(use-package! orderless
:defer t
:init
(setq orderless-component-separator "[ &]"
orderless-matching-styles '(orderless-prefixes
orderless-initialism
orderless-regexp))
:custom
(completion-styles '(orderless))
:config
(custom-set-faces!
'('orderless-match-face-0 :background "#435300" :foreground "#b5bd68")
'('orderless-match-face-1 :background "#4C2E60" :foreground "#b294bb")
'('orderless-match-face-2 :background "#644E00" :foreground "#f0c674")
'('orderless-match-face-3 :background "#1B2E47" :foreground "#81a2be"))
(add-hook 'minibuffer-exit-hook #'orderless-remove-transient-configuration))There was a problem hiding this comment.
I just checked, the orderless completion style and its corresponding completion style functions are indeed marked as autoload. Lazy loading works indeed 👍
|
@loafofpiecrust @jethrokuan I am looking forward to seeing the Selectrum/Consult/Embark suite of packages in Doom. Please let me know if you need anything on the side of Consult. I saw that you are offering a modified project search command here. Maybe some things should be exposed better on the Consult side to reduce the amount of code needed here? You are setting the |
|
fwiw my usage of the selectrum/consult/embark suite has been very minimal, and I'm likely to have gotten many things wrong here. Perhaps someone with more experience can provide a better PR. |
|
FWIW, looks like a selectrum port of ivy/helm-bibtex should be coming soon. cc #2888 |
| (concat (if all-files "-uu") | ||
| (unless recursive "--maxdepth 1") | ||
| "--null --line-buffered --color=always --max-columns=500 --no-heading --line-number" | ||
| " --hidden -g!.git " | ||
| (mapconcat #'shell-quote-argument args " "))) |
There was a problem hiding this comment.
Why is there a space in front of --hidden but not --maxdepth 1 or --null ?
|
Hy @jethrokuan, would you be able to commit a new patch with the requested changes, or do you prefer one of us to take over? |
|
I've been using Selectrum, Orderless, Consult, and Embark for the last couple of weeks, and I'm loving it. I've found a few useful snippets in this pull request and in various repos that I thought I'd share: https://gist.github.com/jcf/ac6f57a5b0c49263619e4d2ee4a0c00a I see Jethro mentioned someone with more experience might want to take the reigns on this pull request. While I'm not sure that's me I'd be happy to help test out any changes, and will keep sharing any useful code I add to my own configuration. Update: in response so Henrik's comment:
This is supported via wgrep with the configuration I shared. Search the project, Update: I've made a few changes to my private config, and I've updated the private gist. There are a few bindings I'm using too. (:when (featurep! :private selectrum)
"*" #'+jcf/search-project-for-symbol-at-point
:desc "Consult find" "SPC" #'consult-find
:desc "Buffers" "," #'consult-buffer
:desc "Modal M-x" ":" #'consult-mode-command
:desc "Re-selectrum" "'" #'selectrum-repeat) |
Do make another pull request, I started this because I was surprised to see that none has been made yet. My time is tied working on org-roam. |
|
@Townk I think it sounds like you might be the best to take over, and then people and PR to your branch. Do you have a link to your personal config btw? |
I'm working on a refactor for my config after my 1000th bankruptcy :P, but you can follow along here: https://github.com/Townk/doom-emacs-private I'm trying a literate config this time. Let's see if I can finish. Regarding the PR, let me work a bit on it today and see where I can get with it. Most likely I will need two PRs in order to get Selectrum fully working, and I won't push anything until later tonight or tomorrow morning (my 2 and 4 yo kids usually get in the way of such things). The parts that mater for this in my config are at "Personal preferences -> Completion framework" and "Personal preferences -> Workspaces". |
| (use-package! orderless | ||
| :when (featurep! +orderless) | ||
| :config | ||
| (setq completion-styles '(orderless))) |
There was a problem hiding this comment.
Here one may consider '(substring orderless) which ensures that TAB prefix completion works under more circumstances. Since substring matches a subset of orderless, this generally works well. Furthermore one could even set '(basic substring orderless), but in this case basic matches strictly less, so this can lead to unexpected behavior but ensures that matches with the current prefix are preferred.
There was a problem hiding this comment.
The current recommended Selectrum+Orderless configuration is as follows, which is compatible with completion styles and optimizes orderless highlighting.
(setq orderless-skip-highlighting (lambda () selectrum-active-p))
(setq selectrum-highlight-candidates-function #'orderless-highlight-matches)
So @emiller88 opened up a new PR for selectrum in #4664. But we need another for the workspace module, to replace persp-mode with perspective. On discord, however, @hlissner suggested "May be better to forgo persp-mode and perspective entirely for tab-bar-mode+desktop." That may be more forward-looking? |
|
@bdarcus Regarding Selectrum and Persp-Mode, I wonder why there is a problem. Selectrum is not doing anything out of the ordinary, relying on nothing else but the minibuffer. Did you open an issue on the perp-mode repository or the Selectrum repository? |
No @minad. I don't understand the details; was just summarizing the status, based on what @Townk concluded. Perhaps he could submit a bug report? Edit: he does explain his conclusion here. I did find that the current code project search functionality doesn't work correctly though. Maybe that has something to do with it? |
|
Closing in favour of #4664 |
Been meaning to try selectrum, decided to package it as a doom module. Still WIP.