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

Missing imports eglot #177

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Missing imports eglot #177

wants to merge 7 commits into from

Conversation

samhedin
Copy link
Collaborator

@samhedin samhedin commented Oct 30, 2020

For #169
Now supports both eglot and lsp. I'm not a huge fan of how I collect the missing imports from eglot. It works by inspecting errors that eglot is about to send to flymake, which makes it a bit hacky. In the lsp-mode version, a call to the function lsp-diagnostics just returns everything that's needed, but I could not find an equivalent function in eglot, though obviously it must be there somewhere.

Werks on my machine, but I need to try it a bit more and have another look at the code.

@brotzeit
Copy link
Owner

brotzeit commented Nov 6, 2020

We still have the problem that we call lsp-mode/flymake code that we didn't require.

@samhedin
Copy link
Collaborator Author

samhedin commented Nov 6, 2020

Good catch, in my mind lsp-mode would return nil when undefined for some reason, but this is clearly not how most languages work, and when I tried it out I must have had lsp-mode loaded by accident. The flymake one is worse though. I want to remove the dependency on it altogether, but the eglot source code is doing so many things I'm unfamiliar with that I've been unable to get it to work with it at all. I might contact the author if I can't come up with anything.

@brotzeit
Copy link
Owner

Maybe we can add files for eglot and lsp-mode and require these files somewhere.

@samhedin
Copy link
Collaborator Author

As it is now, LSP and eglot should not be required if they're not loaded thanks to (ignore-error lsp-mode). Is there another reason to split them up?

Use with 'prefix-arg` to select imports to add."
(interactive)
(when (rustic-cargo-edit-installed-p)
(if-let ((missing-imports (if (ignore-error lsp-mode)
Copy link
Owner

Choose a reason for hiding this comment

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

You can do something like (if (featurep 'lsp-mode) (print "foo"))

(if (string= "E0432" (gethash "code" errortable))
(cons (nth 3 (split-string (gethash "message" errortable) "`")) missing-crates)
missing-crates))
(gethash (buffer-file-name) (lsp-diagnostics t)) '())))
Copy link
Owner

Choose a reason for hiding this comment

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

You can funcall the lsp function here.

@samhedin
Copy link
Collaborator Author

samhedin commented Dec 4, 2020

Just an update, I contacted the eglot maintainers and they'd rather I do something else than the current solution, so I will look into that when I get time, but school is taking up all my focus at the moment.

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