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.

Sign up for GitHub

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

Jump to bottom

Implement crate name autocompletion #120

Merged
merged 6 commits into from
May 12, 2024
Merged

Implement crate name autocompletion #120

merged 6 commits into from
May 12, 2024

Conversation

KaitlynEthylia
Copy link
Contributor

It's not quite perfect just yet, but here's my 1-day implementation of crate name autocompletion as talked about in #115.

You can see it in action here

2024-05-11.18-22-31.mp4

saecki
saecki reviewed May 11, 2024
View reviewed changes
Copy link
Owner

@saecki saecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! There are still a few things to iron out, but I really like where this is going.

lua/crates/config/init.lua Outdated Show resolved Hide resolved
lua/crates/toml.lua Show resolved Hide resolved
lua/crates/toml.lua Outdated Show resolved Hide resolved
lua/crates/toml.lua Outdated Show resolved Hide resolved
@KaitlynEthylia
Copy link
Contributor Author

Lot's of good things pointed out. Unfortunately I've used up my free time for the time being but next week i'll try and iron out what i can, some of these issues are definitely gonna be harder than others

@KaitlynEthylia
rename src to completion
fcd61a9 
`cfg.crate_completion` has also been moved to `cfg.completion.crates`
@KaitlynEthylia
Complete version for unfinished dependency table
cd981f3 
When writing the dependencies in `[dependency.name]` format, the
version row below will now be filled in after the completion is
accepted.

Completions that include the version will also have it as a
snippet instead now.
@KaitlynEthylia
Copy link
Contributor Author

Okay decided to work on this a bit more, this should fix all of these issues. Had to dig into the lsp spec a little bit to get completions to include the version in tables, but i managed to improve a few other things whilst doing so so that worked out great

saecki
saecki reviewed May 12, 2024
View reviewed changes
Copy link
Owner

@saecki saecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The completion rarely works for me. I've found a few things that could be the source of that, buf there seems to be some other issue that I haven't found yet.

lua/crates/completion/common.lua Outdated Show resolved Hide resolved
lua/crates/toml.lua Outdated Show resolved Hide resolved
lua/crates/toml.lua Outdated Show resolved Hide resolved
lua/crates/api.lua Outdated Show resolved Hide resolved
@KaitlynEthylia
Copy link
Contributor Author

Okay as well as fixing the line counts and such, i redid the polling for crates on completions and now it's working loads more reliably!

saecki
saecki reviewed May 12, 2024
View reviewed changes
Copy link
Owner

@saecki saecki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apart from the one comment, I think this is ready to merge.

I can think of two more things to improve crate search completion, but I think those can wait for another PR. The first thing would be a searching virtual text like the loading indicator when fetching crates. The other is an unfinished crate table declaration (without the right ]) not being completed:

[dependencies.abc

But let's first get this merged!

lua/crates/completion/common.lua Outdated Show resolved Hide resolved
@KaitlynEthylia
Copy link
Contributor Author

Virtual text whilst searching is definitely best left for a later date, i've never worked with virtual text before so it's not something I could add in quickly and I'm not sure yet how best to do it. But fixing [dependencies.<name> seemed pretty simple. I'm fairly certain it doesn't break anything, it just makes the toml parser a bit more leniant, which I think is fine, but if you think if should be done a bit more carefully then we can just scrap that last commit and come back to it another day

@saecki
Copy link
Owner

saecki commented May 12, 2024

Thank you very much for this useful addition. ship it 🚀

@saecki saecki merged commit fc63e35 into saecki:main May 12, 2024
2 checks passed
@KaitlynEthylia KaitlynEthylia mentioned this pull request May 12, 2024
Open
@asashakira asashakira mentioned this pull request May 13, 2024
Merged
ueaner added a commit to ueaner/nvimrc that referenced this pull request May 13, 2024
@ueaner
fix(rust): creates.nvim rename src option to completion
4ad742a 
See: saecki/crates.nvim#120
@saecki saecki mentioned this pull request May 13, 2024
Closed
fioncat added a commit to fioncat/ayamir-nvimdots that referenced this pull request May 31, 2024
@fioncat
fix(crates): rename "src" option to "completion"
391aa0b 
The saecki/crates.nvim#120 updated `src` option
to `completion`.
@fioncat fioncat mentioned this pull request May 31, 2024
Merged
CharlesChiuGit pushed a commit to ayamir/nvimdots that referenced this pull request May 31, 2024
@fioncat
Update crates.nvim option name from src to completion (#1272)
870ea55 
The saecki/crates.nvim#120 updated `src` option
to `completion`.
channinghsu pushed a commit to channinghsu/nvim that referenced this pull request Jun 6, 2024
@fioncat @channinghsu
Update crates.nvim option name from src to completion (#1272)
e6689b5 
The saecki/crates.nvim#120 updated `src` option
to `completion`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@saecki saecki saecki left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KaitlynEthylia @saecki