Comments (10)
ye, I was originally thinking it's different from the tree-sitter code as the it doesn't require a C compiler (compared to tree-sitter), but obviously LSP stuff is not very "just Emacs" either.
If eglot
can't find a server, it will most likely ask you to put in an address + port on where to find the server (it essentially assumes you're running a non-standard configuration).
Could be confusing for people that haven't looked into the whole LSP stuff yet.
I feel like this is a pattern we have to figure out long-term for things that require stuff outside of Emacs (compilers, language servers, command-line utils, ...).
Would be nice to also offer a reasonable way to get Crafted Emacs running on a very locked down system where not much is/can be installed.
from crafted-emacs.
Happy to help! I've been testing the changes today against Rust & TS, I'll update the docs in that PR later today 👍
from crafted-emacs.
Thank you for the issue.
If we wouldn't rely on eglot-server-programs
to link the modes, we could do without explicitly loading eglot (as eglot-ensure
is an autoload).
I guess we could provide our own server programs list (which I don't think is a good idea) or just (require 'eglot)
explicitly in the config.
from crafted-emacs.
If we wouldn't rely on
eglot-server-programs
to link the modes, we could do without explicitly loading eglot (aseglot-ensure
is an autoload).I guess we could provide our own server programs list (which I don't think is a good idea) or just
(require 'eglot)
explicitly in the config.
Yeah, I figured as much. I was leaning towards the explicit require approach but wanted to check before submitting a PR since it would ditch the eval-after-load.
I suppose another option could be to make it opt-in with a configuration function similar to the one for tree-sitter, but I personally think there's value in simply requiring it and registering the Eglot-ensure calls.
That said, does Eglot-ensure check for the instance of the LSP server binary before activating? Might lead to a few errors for users who only use LSP for a subset of modes, and don't have all of the prerequisite binaries installed.
from crafted-emacs.
If there is no server for eglot to use, you get an error (error "[eglot] -1 : Server died")
. You still get the language mode loaded however.
lsp-mode
may automatically install a server, there is usually some flag somewhere that lets lsp-mode
know it can be installed. Eglot deliberately does not do this. Thus if the user knows they will be editing Rust files (for example) it is incumbent on them to install the requisite tooling to allow the rust server to start when needed and add whatever configuration eglot needs to connect to that server. If we take the same viewpoint (which we have so far) then we defer to the user to know what they are doing and take the appropriate action / provide appropriate tooling / etc.
To comment on the reason for this issue, we have to wait until after eglot
is loaded because the eglot-server-programs
won't be evaluated until then. However, if we switched to a require
guard instead of eval-after-load
, then the variable would be defined. I see you point about the chicken-and-egg thing. Another possible solution would be for the user to create their own ide module and put 2 lines in it something like this:
;;; my-ide-config.el
(require 'eglot)
(require 'crafted-ide-config)
That may be a bit obtuse, and we can (maybe) safely assume they want eglot turned on with all the hooks and things just because the added the module to their configuration. However, it is possible they only want the tree-sitter configuration and never intend to use eglot
- which means they would never load eglot
and therefore none of the hooks would be set, and they won't get the subsequent "Server died" error every time they visit a language mode buffer.
The end result is, we may need to think a little bit on how we want to approach this for people who want the hooks with eglot-ensure
vs those who do not.
Would be nice to also offer a reasonable way to get Crafted Emacs running on a very locked down system where not much is/can be installed.
Yes! However, that largely precludes the use of either tree-sitter or eglot since both require things to be downloaded - language grammars for TS modes and LSP servers for eglot. So, for those systems, including crafted-ide
is probably not the answer.
from crafted-emacs.
Ah @jeffbowman I think I missed your edits before opening #396. Bringing the discussion back over to this issue.
Anecdotally with that change I had just opened up a markdown buffer and had this pop up:
Error in post-command-hook (#[0 "\303\301!\205�\0r\301q\210\304\305\300\242\306#\210
?\205�\0\307\310\311 \")\207" [(#0) #<buffer list_processing.md> eglot--managed-mode buffer-live-p remove-hook post-command-hook t apply eglot--connect eglot--guess-contact] 4]): (error "None of ’marksman, vscode-markdown-language-server’ are valid executables")
This got me thinking, I personally would not want to install a markdown LSP and it's very annoying to have the error pop up when visiting every markdown file (since Eglot will retry). So maybe this is a mark against my PR and automatically setting this up by default. (Noteworthy that this problem is still present w/o any changes posted, it just requires a user to execute Eglot at some point before the failed server attempts appear.)
With that in mind, I'm wondering a bit more about this feature. Is configuring eglot-ensure
for every known Eglot LSP a useful addition, or should the user be wholly responsible? I do think it's convenient for my configured LSPs to automatically enable, but the list that I personally use is down to 3 or 4 languages.
I'm leaning towards one of these three:
- Don't try to enable
eglot-ensure
for the user at all. - The user opts into
eglot-auto-detect-all
or similar to configureeglot-ensure
for alleglot-server-programs
. This works similarly to the tree sitter configuration in that it requires the user to call the function in their config aftercrafted-ide-config
is loaded. crafted-ide-config
will configureeglot-ensure
out of the box, but only for LSP binaries that exist on the machine (e.g. viaexecutable-find
)
from crafted-emacs.
Is configuring
eglot-ensure
for every known Eglot LSP a useful addition, or should the user be wholly responsible?
Yep, this is the "big money" question. The "simple" (aka naive) options are:
- load them all
- load none of them
We opted for the former especially since the tree-sitter configuration takes the same approach. That side is a bit more reasonable because nothing happens if a language mode is not used, or if the grammar for that language is not present, the TS language mode is also likely missing, so no errors are likely.
... list that I personally use is down to 3 or 4 languages.
Yep, same. And I expect this is likely true for the majority of users. I don't think it's very likely someone is programming in all the languages for which there is an LSP available.
A couple of options I can think of quickly:
- Provide a configuration option to allow the user to supply a list of LSP modes to enable, then use either a
defcustom
initializer to add the hooks for those or adefun
the user would add to their configuration which reads the value of the custom option and adds the hooks. I like the former better. - Provide a
defun
which accepts a list of modes for which to enableeglot
.
Advantages of the former which makes it my preference:
- The option is available through the Easy Customization facilities, which allows the user to customize the option without having to touch lisp.
- The value is saved in the
custom-set-variables
form in thecustom-file
, so it will be loaded the next time Emacs starts. - The user still has the option to write the lisp in their configuration if they choose.
For the latter option:
- We get the same behavioral advantages, ie the hooks set as desired on startup.
- Nothing is saved in the
custom-file
because the configuration is functional instead. - The user must write the lisp in their configuration explicitly, but they are probably already doing that to use Crafted Emacs anyway, so it wouldn't be unexpected.
Regardless which option is developed, the documentation will need to be updated to reflect the change as well - see the docs/crafted-ide.org
file
crafted-ide-config
will configureeglot-ensure
out of the box, but only for LSP binaries that exist on the machine (e.g. viaexecutable-find
)
This is an interesting idea, but it might be quite challenging to maintain long term as we would necessarily need to update executable names, so we would have to track that for every LSP. Not the end of the world, lsp-mode
does most of the work for us for this (sort of) by tracking all the servers on their site - maybe even in the lsp-mode
code as well. So, in spite of liking the idea, I think the trade-off on effort making this a larger lift reduces its attractiveness.
The user opts into
eglot-auto-detect-all
or similar to configureeglot-ensure
for alleglot-server-programs
. This works similarly to the tree sitter configuration in that it requires the user to call the function in their config aftercrafted-ide-config
is loaded.
Yes, I agree with this. One alternative to this option is to provide a defcustom
with an initializer that fires when the option is set.
from crafted-emacs.
This is an interesting idea, but it might be quite challenging to maintain long term as we would necessarily need to update executable names, so we would have to track that for every LSP. Not the end of the world, lsp-mode does most of the work for us for this (sort of) by tracking all the servers on their site - maybe even in the lsp-mode code as well. So, in spite of liking the idea, I think the trade-off on effort making this a larger lift reduces its attractiveness.
I experimented with a patch here to POC what this might look like: eedd8b9
from crafted-emacs.
huh... well, that's way easier than what I was expecting. Have you tested that patch? (probably a silly question)
EDIT: LOL, so you put that in your PR... a POC (to me) is a possible solution that may not have been tested much past the check to make sure the syntax is correct and no compile errors occur... I assume, since it is in your PR, that you have actually tried this code out and it works for you... I'll pull it into a VM and test a bit with it. Thanks for working on it though!
from crafted-emacs.
Tested your POC, unsurprisingly it works well... it was hard to tell if it worked because there is no output either way, but then too, that's just fine. I installed exactly one server and started Emacs, installed all the tree-sitter grammars, restarted Emacs and opened a python file.. and it failed... but apparently pyls
isn't very good quality. Switched to pyright
which worked immediately. Opened a go
file and... nothing... no errors from eglot
. Checked the go-mode-hook
and go-ts-mode-hook
, both were nil
. Checked both python-mode-hook
and python-ts-mode-hook
and both had (eglot-ensure)
.
Thanks for the PR and if you don't mind updating the docs, then I'll merge it. (also mentioned on the PR itself).
Thanks for your work on this!
from crafted-emacs.
Related Issues (20)
- [craftedv2beta] Broken configuration guards (Tracking Issue) HOT 4
- [craftedv2beta] Improve documentation for using straight.el HOT 7
- Provide option for easier module loading. HOT 7
- [craftedv2beta] Use `customize-save-variable` vs. `customize-set-variable`? HOT 11
- [craftedv2beta] Question regarding the skeleton for new modules HOT 5
- Discussion: When do our own `defcustom` variables make sense? When are they best avoided? HOT 6
- [craftedv2beta] Evil bindings in crafted-speedbar are duplicate and inconsistent with evil-collection HOT 3
- [craftedv2beta] Standardized way for naming functions HOT 5
- [craftedv2beta] Unused function `crafted-completion/minibuffer-backward-kill` HOT 2
- texinfo tags in doc headings don't seem to be needed HOT 3
- [v2] Emacs 29+ check for executable prevents Treesitter setup HOT 12
- [v2] No easy way to opt out of tree sitter parsers HOT 5
- [v2] Corfu + Eglot have unsuitable performance for medium/large projects HOT 4
- [v2] defgroup crafted-startup not functioning properly
- [v2]package-selected-packages "Combobulate" problem. HOT 3
- Automating regeneration of Info file HOT 13
- Suggestion: Make examples loadable HOT 3
- Symbol's function definition is void: treesit-auto-add-to-auto-mode-alist HOT 3
- `crafted-init-config` automatically finding directory possibly confounded by submodule use HOT 9
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from crafted-emacs.