Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

General support for narrowing prefixes (used by consult-buffer and consult-flycheck as of now) #55

Merged
merged 26 commits into from
Dec 12, 2020

Conversation

minad
Copy link
Owner

@minad minad commented Dec 11, 2020

I implemented the low-tech consult-buffer solution. I prefix everything with "b#" etc. The prefix is hidden with the display property which is nice. However in order to narrow one has to press "b # SPC" and this is worse than the old behavior. Furthermore there is no support for showing invisible buffers anymore. The consult--buffer-selectrum implementation supported pressing SPC and then only the invisible buffers where shown.

I don't like it. Now I do!

See #10.

Ping @clemera @oantolin

UPDATE: I prefix everything with "b [zero-width-space]". Then I modify the keymap such that if pressing space after a prefix character like "b", it inserts "[zero-width-space] SPC". This works nicely, allows widening by deleting the prefix characters again etc. I am happy with the solution. It is reasonably low-tech. No dynamic candidates. Only the keymap tweak.

@oantolin
Copy link
Contributor

oantolin commented Dec 11, 2020

Maybe it is good enough. It certainly is good enough for orderless users. Orderless is comically configurable, you can configure it so a string of the form b#rest (without a space after b#) matches the b# at the start and the rest in any way you wish. For example, you could turn b#rest into the regexp ^b#.*\(rest\).

Of course, you don't want to cater just to Orderless users.

EDIT: (If you really wanted it to be b rest, that can be configured in orderless, too. Not sure why I used # for the example.)

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@oantolin I found something better - I just implemented the space hack.

(You used b#, because this is what I used before)

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@oantolin Could you help me as the local actions expert - what is the best way to bind the SPC key only locally for the current command? Use minibuffer setup and exit hooks and define/undefine there? But this is not compatible with recursion right? Or can I only modify the keymap locally somehow?

@oantolin
Copy link
Contributor

So you removed the ^ from the latest version, right? I was going to say you shouldn't count on the completion style understanding ^ since none of the ones that come with Emacs do.

@oantolin
Copy link
Contributor

@oantolin Could you help me as the local actions expert - what is the best way to bind the SPC key only locally for the current command? Use minibuffer setup and exit hooks and define/undefine there? But this is not compatible with recursion right? Or can I only modify the keymap locally somehow?

I don't know! This would be good to figure out. Embark never needed this since the funky keybindings don't need to be active for the entire minibuffer completion session, only immediately after running embark-act.

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

So you removed the ^ from the latest version, right? I was going to say you shouldn't count on the completion style understanding ^ since none of the ones that come with Emacs do.

Yes, I removed that again. This does not work well. Sorry for the confusion.

@oantolin
Copy link
Contributor

I think the setup and exit hooks are your best bet. With some extra local variables to keep track of state you can get it right on recursive minibuffers too.

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@oantolin I added an implementation which seems to work. I think this approach is acceptable. What do you think in general about this method to implement narrowing?

The nice thing is that it can possibly generalize to arbitrary other candidates and is reasonably low-tech. You only need the setup and add a prefix to the candidates.

@minad minad force-pushed the low-tech-consult-buffer branch 3 times, most recently from 1733646 to fb7996e Compare December 11, 2020 19:15
@minad minad changed the title Low tech consult buffer General support for narrowing prefixes (used by consult-buffer and consult-flycheck as of now) Dec 11, 2020
@clemera
Copy link
Contributor

clemera commented Dec 11, 2020

I tested with selectrum and it wokrs great after I found out how to set the keymap ;) Congrats for getting a general solution working! consult-narrow-separator has to be unique string not appearing in the candidates, is that how it works?

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@clemera Thanks. Yes, see the UPDATE above and the docstring. I am using zero-width-space for now. It is weird when narrowing widening since you have to press backspace twice, but besides that it looks nice and zero-width-space won't normally occur in candidates.

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

Now I am waiting for @purcell to sync with MELPA, see #54. After that I will merge this PR and consult will be based 100% on the completing-read API! 🥳

@clemera
Copy link
Contributor

clemera commented Dec 11, 2020

I am using zero-width-space for now. It is weird when narrowing widening since you have to press backspace twice, but besides that it looks nice and zero-width-space won't normally occur in candidates.

Yes that's a nice idea. Though for me I see ZWSP box because I have things setup so I see format-control chars (via glyphless-char-display-control) but that is probably uncommon.

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

Ok, in that case I suggest to use for example the vertical line character. Or some unicode colon? Therefore I made that configurable.

@clemera
Copy link
Contributor

clemera commented Dec 11, 2020

Yeah, I will think about it, maybe it's not to bad to have a visible char like an arrow or something, than you also see you need to delete it.

@oantolin
Copy link
Contributor

It works great with icomplete too! It should probably support the default completion as well, the relevant map would be minibuffer-local-completion-map.

You have a typo: in consult--icomplete-preview-setup there is a icomplete-post-commanda-hook with an extra a after command.

@oantolin
Copy link
Contributor

Ok, in that case I suggest to use for example the vertical line character. Or some unicode colon? Therefore I made that configurable.

I like , which is "VERTICAL ELLIPSIS".

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@clemera @oantolin Do you think it is better to use the ellipsis by default? It is probably less confusing. And I can still document that the zero width space is also a good setting.

@clemera
Copy link
Contributor

clemera commented Dec 11, 2020

I think either way would be fine though as you say the more visible approach could be less confusing for first time users.

@oantolin
Copy link
Contributor

I was going to say the same thing as @clemera. The ellipsis is slightly less confusing, but I don't think the zero width space is actually very confusing and would be fine too. I do know that I wouldn't bother changing the default, so I know I'm OK with either one.

@minad
Copy link
Owner Author

minad commented Dec 11, 2020

@oantolin @clemera I tried the vertical ellipsis and unfortunately with the fonts I tested it didn't always look nice. Therefore I am more inclined to keep the zero-width-space as default now.

@minad minad merged commit 4fe0890 into master Dec 12, 2020
@minad minad deleted the low-tech-consult-buffer branch December 12, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants