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

Handle completion of % and # completion correctly. #83

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

iteratee
Copy link

The current behavior if you try to complete :e %:p:h or something similar replaces only the h with the path, which isn't the desired behavior, because then nvim tries to open the wrong filename. Detect this case and produce edits that will replace the entire % chain with modifiers.

@iteratee
Copy link
Author

My solution may not be ideal. I would be happy to re-write it given some pointers. I'm new to lua and vim plugins.

@aldevv
Copy link

aldevv commented Jun 9, 2023

any plans on merging this? this problem is been a pain for a while @hrsh7th

@fent
Copy link

fent commented Aug 19, 2023

please :(

@iteratee
Copy link
Author

iteratee commented Jan 5, 2024

I'll try and rebase my current solution and push it up.

@iteratee iteratee force-pushed the kb/file-name-completion branch from da5fdaf to 8cb39fd Compare January 5, 2024 23:07
@0e4ef622
Copy link

0e4ef622 commented Mar 4, 2024

I am installing from this PR using the commit hash, but it would be nice to have this merged.

@FoamScience
Copy link

In visual mode, completion doesn't keep track of typed chars, so generates wierdy items.
Fixed and rebased in iteratee#1 in case that takes some time to get merged, you can install from that PR instead

The current behavior if you try to complete :e %:p:h or something similar
replaces only the h with the path, which isn't the desired behavior, because
then nvim tries to open the wrong filename. Detect this case and produce edits
that will replace the entire `%` chain with modifiers.
Ignore fixed_input and use arglead directly in the case of a magic file
match. This unbreaks `%:.:h:h`, while still handling fixed_input
correctly in the non magic_file case.
@iteratee iteratee force-pushed the kb/file-name-completion branch from 589b31f to 7e31674 Compare April 24, 2024 20:07
@liskin
Copy link

liskin commented Sep 1, 2024

I tried this "fix" and it's not working well for me, it replaces the space before the % as well. Inspired by #110 (which fixes wildcards and %/# and breaks almost everything else), I believe this can be improved by vastly simplifying the code to just:

        if is_magic_file then
          item.insertText = item.label
          item.label = arglead .. '' .. item.label
        end

All the complicated range stuff can be dropped.


Still, this is just a workaround, not an actual fix. Expansion of file wildcards is still broken, as are many other usecases where the fixed_input heuristics don't find the same offset as nvim's completion does internally.

The root cause of why this can't be fixed properly here is that getcompletion() doesn't expose the column where the returned matches are meant to be inserted. The person who added the ability to get cmdline completion from getcompletion() deals with this in their own funny way: https://github.com/Shougo/ddc-source-cmdline/blob/c03488e9239cba6c3bfbc000b17677dd47278e5a/denops/%40ddc-sources/cmdline.ts#L72-L86
But this can never work reliably enough. The correct way to deal with this is to extend getcompletion() to have a similar interface to omnifunc: https://github.com/vim/vim/blob/3d5065fc7553c8de3d0555c3085165bdd724663b/runtime/doc/insert.txt#L1105-L1122, so we can get the actual column where the matches are meant to be inserted without guessing.

Personally, I gave up and went back to nvim's builtin cmdline completion. nvim-cmp + cmp-cmdline didn't provide enough added value for me to sink additional hours into submitting a PR to neovim and then running a patched version until it makes it into a release and that release makes it to Debian. If someone else wants to pursue fixing this proper, feel free to get in touch for guidance.

@liskin liskin mentioned this pull request Sep 1, 2024
liskin added a commit to liskin/dotfiles that referenced this pull request Sep 1, 2024
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.

6 participants