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

Override faces from completion styles with our own in preview #533

Closed
wants to merge 1 commit into from

Conversation

wyuenho
Copy link
Contributor

@wyuenho wyuenho commented Dec 8, 2024

Some completion styles like corfu-prescient will apply faces to the candidate strings deep inside the completion-in-region machinery before corfu gets to them. While these faces are useful in the completion popup, they aren't so useful in the preview. This is evident in the first screenshot in the README, here's another example in TypeScript where they make even less sense.

ScreenRecording2024-12-08at5 49 27PM-ezgif com-optipng

This PR makes a copy of the candidate string and overrides the face property with a new face that by default inherits from corfu-default, so the user can customize the face if they so choose.
ScreenRecording2024-12-08at5 58 11PM-ezgif com-optipng

@minad
Copy link
Owner

minad commented Dec 8, 2024

This problem does neither occur with the built-in completion styles like basic or substring nor with orderless. Please report the issue to corfu-prescient. I suspect the problem is that corfu-prescient doesn't properly support completion-lazy-hilit. It would be great to support it there, since this is also important for performance.

@minad minad closed this Dec 8, 2024
@wyuenho
Copy link
Contributor Author

wyuenho commented Dec 8, 2024

This does happen with orderless, although, after some more testing, this seems to only happen to CAPF functions that return a category in the metadata, where the category contains the flex style. Which means, if one is using the built-in eglot version in Emacs 29, is susceptible to this bug. If one follows the wiki's recommendation for LSP mode, one is also susceptible to this bug. In fact, if one overrides any of the categories for any of the CAPE functions to include a flex style, they'll run into this bug.

So, for safety, I'd recommend you merge this PR. There's no harm in doing so and saves a lot of people's time debugging and playing around with completion style overrides.

@minad
Copy link
Owner

minad commented Dec 8, 2024

You are right, the flex style indeed doesn't use lazy highlighting in Emacs 29 and older. I pushed a fix.

@wyuenho
Copy link
Contributor Author

wyuenho commented Dec 8, 2024

That also works. Let's see how well it works. Thanks.

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.

2 participants