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

Refactor eglot--maybe-activate-managed-mode #942

Closed
wants to merge 7 commits into from

Conversation

rbrtb
Copy link
Contributor

@rbrtb rbrtb commented Apr 30, 2022

Fixes #824

eglot.el Show resolved Hide resolved
@rbrtb rbrtb marked this pull request as draft May 1, 2022 06:49
@rbrtb rbrtb marked this pull request as ready for review May 1, 2022 07:14
@rbrtb rbrtb changed the title Refactor eglot--managed-mode enabling Refactor eglot--maybe-activate-managed-mode May 1, 2022
@joaotavora
Copy link
Owner

OK. But instead of adding structural changes to eglot.el, how about describing exactly the use case that you're thinking of solving?

Earlier, I had the impression that your use case concerns usage of eglot-ensure in a specific scenario, which isn't perfectly described (as in a formal bug report that I myself can witness). So why not do one of these things, or both:

  • describe the minimal reproducible example that shows evidence of the bug
  • think about touching eglot-ensure?

@rbrtb
Copy link
Contributor Author

rbrtb commented May 2, 2022

When magit created a non-file buffer(such as magit-blob-previous on a git tracked file),
it will temporarily set buffer-file-name for the usage of normal-mode:

(let ((buffer-file-name magit-buffer-file-name)
          (after-change-major-mode-hook
           (remq 'global-diff-hl-mode-enable-in-buffers
                 after-change-major-mode-hook)))
      (normal-mode t))

Thereby eglot--managed-mode is enabled on that buffer.
Then when you move around in that buffer, eldoc is triggered, and there's error: (wrong-type-argument arrayp nil).

@rbrtb
Copy link
Contributor Author

rbrtb commented May 2, 2022

And if you think current changes are too structural, perhaps consider adding an option to toggle automanage? 6900927

@joaotavora
Copy link
Owner

it will temporarily set buffer-file-name for the usage of normal-mode:

Magic lying then, right? It says there is a supporting file, but there is none. I can't tell if this is the problem from your short description. I also have no idea what normal mode is.

Also your current implementation breaks all Eglot tests.

Adding a toggle to something as fundamental as Eglot's most basic working principle sounds more complicated than perhaps asking Magic matters why they do those things.

@rbrtb
Copy link
Contributor Author

rbrtb commented May 2, 2022

Well I agree that it's more of a magit problem, so I'm closing this.

@rbrtb rbrtb closed this May 2, 2022
@rbrtb rbrtb deleted the eglot--managed-mode branch May 2, 2022 18:36
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.

Error when buffer is not visiting a file
2 participants