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

#1106 #340 Complete inside symbols #1474

Merged
merged 29 commits into from
Jul 13, 2024
Merged

#1106 #340 Complete inside symbols #1474

merged 29 commits into from
Jul 13, 2024

Conversation

dgutov
Copy link
Member

@dgutov dgutov commented Jun 5, 2024

This aims to resolve #340 and #1106.

Idle completion inside a symbol works automatically, disabled by the new option company-inhibit-inside-symbols.

Completion backends in their prefix handler now can return a list (PREFIX SUFFIX &optional PREFIX-LEN-OVERRIDE) instead of previous forms PREFIX or (PREFIX . LEN-OVERRIDE) which continue to work, but are softly obsolete.

A backend command adjust-boundaries is added, similar in effect to completion-boundaries.

company-capf has implementations for both, improving support for our CAPF integrations - e.g. difference is visible in M-x shell, where completions are shorter now - base names like in the default UI (not full names). And using company-capf will usually filter by suffix, and replace the suffix at the end of completion. With the exception of (currently hardcoded) completion style emacs22: when the other styles don't match (no completions match the suffix), this style is used - and it doesn't filter by suffix, nor replace it.

User options company-dabbrev-code-completion-styles and company-etags-completion-styles, when turned on, make the corresponding two backends behave the same in these respects.

The default behavior (when a backend doesn't override the logic) is simpler. To try to strike a balance, the suffix is matched with string-suffix-p - this affects the highlighting in the popup but not the filtering of completions (or sorting). When the suffix matches, it's replaced at the end of completion (and kept otherwise). But this is up for discussion - e.g. we could adopt the behavior from the basic completion style a well, where the suffix is matched as substring.

Examples below.

Without completion styles

Suppose for input add|Prop ▶️ B you have completions addBarProp, addBaaa, and addBaaaProperty.

Choosing the first will lead to addBarProp|. Second will lead to addBaaa|Prop. Third will lead to addBaaProperty|Prop.

With completion styles

To use an example with work delimiters so "partial completion" can be used too.

For input add-|pr ▶️ f, completions add-footure-prop, add-foo-bar-pr. All replace the suffix: add-footure-prop|.

If for input add-|pr ▶️ f there are no completions with pr inside: only add-foo and add-footure - then completion keeps the suffix: add-foo|pr.

More options could be added, but for most it can be enough to customize completion-styles or company-dabbrev-code-completion-styles, I think.

For now the main strategy is "replace suffix when it matches completion".

Probably add an option later.
@dgutov dgutov mentioned this pull request Jun 6, 2024
@nfarlow-js
Copy link

nfarlow-js commented Jun 17, 2024

Hi! I'm hoping to leverage this PR as in the following screenshot:

image

In this screenshot, the prefix would be the first part of the line (let items = [ ), and the suffix would be the last part of the line (]). Ideally, I'd like the preview to hide the suffix, and I'd like the suffix to be removed when the completion is accepted, even though the completion doesn't match the suffix. Can this be supported?

@dgutov
Copy link
Member Author

dgutov commented Jun 18, 2024

@nfarlow-js Hi! Yes, the final version should be able to support it.

Ideally, I'd like the preview to hide the suffix

I think your picture shows the suffix as visible, doesn't it? That's the closing bracket at the end.

and I'd like the suffix to be removed when the completion is accepted, even though the completion doesn't match the suffix. Can this be supported?

Definitely can be.

In your example, if you don't actually want the prefix hidden during completion, this example could be implemented more easily by not including the closing bracket. You'd just return an empty string as suffix.

Otherwise: are you using a native backend (or planning to, or are open to it), or are you using a CAPF function (or planning to)?

With the latter I haven't yet decided on the strategy - the main thing I currently know is some users want it the prefix to stay, and some - such as yourself - want it removed. It should probably be decided by the completion style, but on average that would require a more complex setup, code-wise.

@nfarlow-js
Copy link

Thanks for the response!

I think your picture shows the suffix as visible, doesn't it? That's the closing bracket at the end.

Sorry for the confusion; the screenshot shows how the completion looks right now using the current code changes in this PR, not what the completion should look like ideally. You're right, the suffix is visible in the screenshot, but I don't want it to be, since in my completion backend, I know that the suffix will always be subsumed by the completion. Specifically, when I request a completion in the middle of a line, I know that I receive a completion that should replace the end of the line with one or more lines.

Otherwise: are you using a native backend (or planning to, or are open to it), or are you using a CAPF function (or planning to)?

This completion is implemented using a CAPF function.

With the latter I haven't yet decided on the strategy - the main thing I currently know is some users want it the prefix to stay, and some - such as yourself - want it removed

In my case, I'm happy to keep the prefix; it's the suffix I'd like to hide during the preview (since the completion subsumes it) and replace when the completion is accepted.

Hopefully this clears some things up!

@dgutov
Copy link
Member Author

dgutov commented Jun 18, 2024

In my case, I'm happy to keep the prefix; it's the suffix I'd like to hide during the preview (since the completion subsumes it) and replace when the completion is accepted.

Sorry, I meant the suffix, of course.

You're right, the suffix is visible in the screenshot, but I don't want it to be, since in my completion backend, I know that the suffix will always be subsumed by the completion.

Yeah, now that I look at the picture the corresponding closing bracket in the completion is the one after "3" - so it would make sense to remove the suffix.

Thanks for the answers.

dgutov added 14 commits June 25, 2024 04:20
Unless the completion style ignores the suffix (the only currently known example
in emacs22, so we hardcode it, for now).

#340
#1106
Though only when company-dabbrev-code-completion-styles is non-nil.

#340
#1106
Support CAPF's boundaries through it.

The special emacs22's style behavior also goes through it.

CAPF completion tables using boundaries (such as the one in 'M-x shell') behave
more "natively".

#1106 #340 #426
And add popup highlighting for success (when completion will replace it).
Rather than cutting the overlay itself shorter.

#1106 #340
@dgutov
Copy link
Member Author

dgutov commented Jul 7, 2024

@nfarlow-js

it's the suffix I'd like to hide during the preview (since the completion subsumes it) and replace when the completion is accepted

The latest version in this branch now has this.

@dgutov
Copy link
Member Author

dgutov commented Jul 7, 2024

@vspinu @zk-phi @jscheid @yugaego, you commented in the linked issues. Feedback welcome - perhaps you see any problems, or would prefer different defaults. No pressure, it's really been a while.

EDIT: See the description at the top.

@dgutov
Copy link
Member Author

dgutov commented Jul 7, 2024

Also something of note: lsp-mode and eglot behave differently wrt suffix.

lsp-completion-at-point always returns an empty suffix (end=point), meaning the suffix is never used to filter, and is never replaced at the end. eglot-completion-at-point behaves more classicaly (suffix is both matched and replaced).

I've never seen it mentioned in comparative reviews, so the exact choice might be not important (?).

E.g. also after delete-forward-char.
No support for spaces in the suffix (escaping is hard).

#340
The saved "boundary cons" is likely outdated as well. And there seems to be no
way to fetch adjusted boundaries outside of a `candidates` call.

#340
@dgutov dgutov merged commit 31f7ad5 into master Jul 13, 2024
7 checks passed
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.

Enable inside symbols?
2 participants