* bug#64784: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] [not found] ` <875y6bm5ut.fsf@fastmail.fm> @ 2023-07-23 10:20 ` João Távora 2023-07-23 15:13 ` Theodor Thornhill 2023-07-24 17:03 ` Tassilo Horn 0 siblings, 2 replies; 8+ messages in thread From: João Távora @ 2023-07-23 10:20 UTC (permalink / raw) To: Tassilo Horn; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel Tassilo Horn <tsdh@gnu.org> writes: >> Tassilo, can you test? > I can say at least that the error is gone. Thank you! In that case, I'm closing, but we should keep chatting because this matter interests me as well. In fact, let's move this to Emacs-devel. > So is there a way to stop the indentation wars [between LSP and Emacs's]? There's quite a bit of overlap in indentation functionality, yes. For example, in non-ts c++-mode, there are a lot of indentation knobs, and they can probably do all this. But configuring them is difficult (for me), and I'm not sure I haven't seen at a bug or two. In practice, I've accepted I'll never get them to match my team's .clang-format completely. A way to "stop" the war is to get one of the sides to surrender. To make the LSP side lose, just add symbols to 'eglot-ignored-server-capabilities', like ':documentOnTypeFormattingProvider'. > Maybe if I could make it so that return and tab would also be > considered as trigger characters for eglot-format? If your aim is to make the LSP side "win", I don't think you should use the "trigger character" technique specifically. But in Emacs you can of course bind keys to commands that invoke 'eglot-format' synchronously. Even better, I think the most correct way is to buffer-locally set 'indent-line-function' and 'indent-region-function', so you can keep the familiar feeling of TAB. I've tested this: * Setting 'indent-region-function' simply to 'eglot-format' apparently works. (Not in cc-mode, which has a tendency for wheel-reinvention. Or who knows original invention... but in any case it's probably time for me to move on to c++-ts-mode) * As to 'indent-line-function', there's no Eglot command that's exactly compatible with the protocol, but it's pretty easy to make one: (defun eglot-indent-line () (eglot-format (line-beginning-position) (line-end-position))) After setting 'indent-line-function' and 'indent-region-function' like this, things seem to work well at first. But simple things like RET ('newline') fail. I haven't investigated. Maybe it's becasue of electric-indent-mode? Or maybe just because of the fact that `eglot-format` asks the language server to do more than just indenting, namely it also inserts newlines. Or maybe I'm just not passing in the correct range. And then there are the annoying messages in the echo area about edits successfully applied, but that's easily solved. I hope you (and/or others) can give this approach (or variations thereof) some testing. Maybe with other LSP servers and/or style files. The reason I find this interesting it that it would IMO not only solve the indentation wars, it solves fundamental problems of limited-context indenters such as c++-ts-mode. Consider the C/C++ textual preprocessor macros: probably no tree-sitter mode can know exactly what they mean and how to indent the surrounding code, but a sufficiently smart project-knowing LSP can. Are there drawbacks to this "LSP-wins" approach to indentation? Probably. Chief among them LSP is very slow when compared things in the same address space, like an Elisp function or a dynamically linked C library. My early impression is that this fact almost certainly matters for LSP-driven fontification, but not for indentation. In summary, if we can get a successful approach that feels right for Emacs users (mostly regarding TAB, newline and region-indenting), maybe we can enshrine LSP-powered indentation in Eglot-managed buffers just like we do for Xref, Flymake, Imenu, etc... João ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] 2023-07-23 10:20 ` bug#64784: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] João Távora @ 2023-07-23 15:13 ` Theodor Thornhill 2023-07-23 16:42 ` João Távora 2023-07-24 17:03 ` Tassilo Horn 1 sibling, 1 reply; 8+ messages in thread From: Theodor Thornhill @ 2023-07-23 15:13 UTC (permalink / raw) To: João Távora, Tassilo Horn Cc: Eli Zaretskii, Stefan Monnier, emacs-devel João Távora <joaotavora@gmail.com> writes: > Tassilo Horn <tsdh@gnu.org> writes: > >>> Tassilo, can you test? >> I can say at least that the error is gone. Thank you! > > In that case, I'm closing, but we should keep chatting because this > matter interests me as well. In fact, let's move this to Emacs-devel. > >> So is there a way to stop the indentation wars [between LSP and Emacs's]? > > There's quite a bit of overlap in indentation functionality, yes. For > example, in non-ts c++-mode, there are a lot of indentation knobs, and > they can probably do all this. But configuring them is difficult (for > me), and I'm not sure I haven't seen at a bug or two. In practice, I've > accepted I'll never get them to match my team's .clang-format > completely. > > A way to "stop" the war is to get one of the sides to surrender. To > make the LSP side lose, just add symbols to > 'eglot-ignored-server-capabilities', like > ':documentOnTypeFormattingProvider'. > >> Maybe if I could make it so that return and tab would also be >> considered as trigger characters for eglot-format? > > If your aim is to make the LSP side "win", I don't think you should use > the "trigger character" technique specifically. But in Emacs you can of > course bind keys to commands that invoke 'eglot-format' synchronously. > > Even better, I think the most correct way is to buffer-locally set > 'indent-line-function' and 'indent-region-function', so you can keep the > familiar feeling of TAB. > > I've tested this: > > * Setting 'indent-region-function' simply to 'eglot-format' apparently > works. (Not in cc-mode, which has a tendency for wheel-reinvention. > Or who knows original invention... but in any case it's probably time > for me to move on to c++-ts-mode) > > * As to 'indent-line-function', there's no Eglot command that's exactly > compatible with the protocol, but it's pretty easy to make one: > > (defun eglot-indent-line () > (eglot-format (line-beginning-position) (line-end-position))) > > After setting 'indent-line-function' and 'indent-region-function' like > this, things seem to work well at first. But simple things like RET > ('newline') fail. I haven't investigated. Maybe it's becasue of > electric-indent-mode? Or maybe just because of the fact that > `eglot-format` asks the language server to do more than just indenting, > namely it also inserts newlines. Or maybe I'm just not passing in the > correct range. > > And then there are the annoying messages in the echo area about edits > successfully applied, but that's easily solved. > > I hope you (and/or others) can give this approach (or variations > thereof) some testing. Maybe with other LSP servers and/or style files. > > The reason I find this interesting it that it would IMO not only solve > the indentation wars, it solves fundamental problems of limited-context > indenters such as c++-ts-mode. Consider the C/C++ textual preprocessor > macros: probably no tree-sitter mode can know exactly what they mean and > how to indent the surrounding code, but a sufficiently smart > project-knowing LSP can. > > Are there drawbacks to this "LSP-wins" approach to indentation? > Probably. Chief among them LSP is very slow when compared things in the > same address space, like an Elisp function or a dynamically linked C > library. > > My early impression is that this fact almost certainly matters for > LSP-driven fontification, but not for indentation. > > In summary, if we can get a successful approach that feels right for > Emacs users (mostly regarding TAB, newline and region-indenting), maybe > we can enshrine LSP-powered indentation in Eglot-managed buffers just > like we do for Xref, Flymake, Imenu, etc... > > João (I haven't read that bugreport yet, sorry if this has already been discussed) This is interesting! But for the time being many lsp-servers vary a lot in their ability to do this properly. Gopls, for example, seems to handle this very well, in that already has a very heavily enforced indentation style. Same goes for rust. Java, however, is a different beast. Intellij has its own indentation implementation that isn't lsp-available. Eclipse is lsp-available, but was for me almost impossible to configure in the initializationOptions, as it required sourcing a file with xml-settings etc. Getting jdtls to format things like Intellij, which coworkers use, is hard. Similarly, for javascript, where other tools, like prettier, is used to do this. Sometimes they allow for a plugin to the lsp-server, sometimes it has to be executed separately. My point is that getting indentation correct often relies on factors outside of lsp, or emacs even, making it hard to configure. VSCode offers a settings-ui for much of this, which writes to its own settings.json (or something like that). I'm not sure how such a configuration would look for emacs, but I'm sure it's possible. Theo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] 2023-07-23 15:13 ` Theodor Thornhill @ 2023-07-23 16:42 ` João Távora 0 siblings, 0 replies; 8+ messages in thread From: João Távora @ 2023-07-23 16:42 UTC (permalink / raw) To: Theodor Thornhill Cc: Tassilo Horn, Eli Zaretskii, Stefan Monnier, emacs-devel On Sun, Jul 23, 2023 at 4:13 PM Theodor Thornhill <theo@thornhill.no> wrote: > This is interesting! But for the time being many lsp-servers vary a lot > in their ability to do this properly. Gopls, for example, seems to > handle this very well, in that already has a very heavily enforced > indentation style. Same goes for rust. Java, however, is a different > beast. Intellij has its own indentation implementation that isn't > lsp-available. Eclipse is lsp-available, but was for me almost > impossible to configure in the initializationOptions, as it required > sourcing a file with xml-settings etc. Getting jdtls to format things > like Intellij, which coworkers use, is hard. Similarly, for javascript, > where other tools, like prettier, is used to do this. Sometimes they > allow for a plugin to the lsp-server, sometimes it has to be executed > separately. My point is that getting indentation correct often relies on > factors outside of lsp, or emacs even, making it hard to > configure. So do many other aspects of LSP already. Uniform configuration isn't LSP's strong suit. And as you mention each team/language server usually has its own underlying rule format and, as you mention, sometimes it's not even possible to plug it into LSP. Eglot users of those subpar language servers can always opt for good ol Emacs rules using eglot-ignored-server-capabilities or something else (IMO if a server can't do something well, it simply shouldn't announce it can, and Eglot can just detect that). So right now, I'm not really worried about this: I'm more interested in how well LSP's "format" abstraction meshes with Emacs's "indent" abstractions. I want to find out if we can get a good match in terms of performance and functionality, and I'm primarily interested in "good" servers (gopls, rust-analyzer, clangd,...). IOW, I'm calling on Eglot tinkerers to propose and test something to set indent-* variables to around lines 1870-1950 of lisp/progmodes/eglot.el :-) João ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] 2023-07-23 10:20 ` bug#64784: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] João Távora 2023-07-23 15:13 ` Theodor Thornhill @ 2023-07-24 17:03 ` Tassilo Horn 2023-07-24 18:13 ` Theodor Thornhill via Emacs development discussions. 1 sibling, 1 reply; 8+ messages in thread From: Tassilo Horn @ 2023-07-24 17:03 UTC (permalink / raw) To: João Távora; +Cc: Eli Zaretskii, Stefan Monnier, emacs-devel João Távora <joaotavora@gmail.com> writes: Hi João, > If your aim is to make the LSP side "win", Yes, please! > I don't think you should use the "trigger character" technique > specifically. But in Emacs you can of course bind keys to commands > that invoke 'eglot-format' synchronously. > > Even better, I think the most correct way is to buffer-locally set > 'indent-line-function' and 'indent-region-function', so you can keep > the familiar feeling of TAB. I've now tried this: (defun th/eglot-indent-line () (eglot-format (line-beginning-position) (line-end-position))) (defun th/eglot-format-setup () (setq-local indent-region-function #'eglot-format) (setq-local indent-line-function #'th/eglot-indent-line)) (add-hook 'eglot-managed-mode-hook #'th/eglot-format-setup) Basically, it works, but it seems rust-analyzer doesn't support formatting of only a range. eglot--error: [eglot] Unsupported or ignored LSP capability `:documentRangeFormattingProvider' No big deal, so now I tried just using eglot-format also an indent-line-function. But indeed, then I cannot insert newlines anymore. :-) I'll try experimenting a bit more at some time. Bye, Tassilo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] 2023-07-24 17:03 ` Tassilo Horn @ 2023-07-24 18:13 ` Theodor Thornhill via Emacs development discussions. 2023-07-27 21:51 ` LSP vs Emacs indentation [ Stephen Leake 0 siblings, 1 reply; 8+ messages in thread From: Theodor Thornhill via Emacs development discussions. @ 2023-07-24 18:13 UTC (permalink / raw) To: Tassilo Horn, João Távora Cc: Eli Zaretskii, Stefan Monnier, emacs-devel Tassilo Horn <tsdh@gnu.org> writes: > João Távora <joaotavora@gmail.com> writes: > > Hi João, > >> If your aim is to make the LSP side "win", > > Yes, please! > >> I don't think you should use the "trigger character" technique >> specifically. But in Emacs you can of course bind keys to commands >> that invoke 'eglot-format' synchronously. >> >> Even better, I think the most correct way is to buffer-locally set >> 'indent-line-function' and 'indent-region-function', so you can keep >> the familiar feeling of TAB. > > I've now tried this: > > (defun th/eglot-indent-line () > (eglot-format (line-beginning-position) (line-end-position))) > > (defun th/eglot-format-setup () > (setq-local indent-region-function #'eglot-format) > (setq-local indent-line-function #'th/eglot-indent-line)) > > (add-hook 'eglot-managed-mode-hook #'th/eglot-format-setup) > > Basically, it works, but it seems rust-analyzer doesn't support > formatting of only a range. > > eglot--error: [eglot] Unsupported or ignored LSP capability `:documentRangeFormattingProvider' > > No big deal, so now I tried just using eglot-format also an > indent-line-function. But indeed, then I cannot insert newlines > anymore. :-) > > I'll try experimenting a bit more at some time. > One issue with this is that most/many formatters remove whitespace, and for indentation _inserting_ whitespace is paramount. Consider (| is the cursor) ``` func foo() {|} ``` Now if you type RET we'd expect some incantation of ``` func foo() { | } ``` to be the expected output, not the cursor at col 0, which is what happens now. That means we'd have to do something like ``` (defun eglot-indent-line () (eglot-format (line-beginning-position) (line-end-position)) (eglot-newline-and-indent-according-to-mode)) ``` Where the 'eglot-newline-and-indent-according-to-mode' has to calculate the expected indentation. I don't understand how we'd expect the formatters to do that indentation for us. They only care about code already written, not code yet to be written. So if we'd have to calculate that offset anyway, do we win much? How about a hybrid approach, where eglot can take care of the formatting part, but the "move cursor to indentation of parent + N spaces" is handled by the respective major modes? This would make this contrived example work: ``` func foo() { foo()| foo() foo() } ``` Now type RET, and output of the file would be: ``` func foo() { foo() | foo() foo() } ``` or ``` func foo() { if err != nil {|} } ``` Now type RET, and output of the file would be: ``` func foo() { if err != nil { | } } ``` The placement of the wrongly indented function calls are formatted by eglot, and the indentation of the blank line is handled by emacs. This diff will show a very naive example implementation of this. Thanks, Theo @@ -1816,9 +1816,16 @@ 'eglot--ensure-list \f ;;; Minor modes ;;; + +(defun eglot-electric-newline () + (interactive) + (eglot-format) + (newline-and-indent)) + (defvar eglot-mode-map (let ((map (make-sparse-keymap))) (define-key map [remap display-local-help] #'eldoc-doc-buffer) + (define-key map [remap newline] #'eglot-electric-newline) map)) (defvar-local eglot--current-flymake-report-fn nil ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [ 2023-07-24 18:13 ` Theodor Thornhill via Emacs development discussions. @ 2023-07-27 21:51 ` Stephen Leake 2023-07-28 5:28 ` Tassilo Horn 0 siblings, 1 reply; 8+ messages in thread From: Stephen Leake @ 2023-07-27 21:51 UTC (permalink / raw) To: Theodor Thornhill via Emacs development discussions. Cc: Tassilo Horn, João Távora, Theodor Thornhill, Eli Zaretskii, Stefan Monnier Theodor Thornhill via "Emacs development discussions." <emacs-devel@gnu.org> writes: > Consider (| is the cursor) > > ``` > func foo() {|} > ``` > > Now if you type RET we'd expect some incantation of > > ``` > func foo() { > | > } > ``` > > to be the expected output, not the cursor at col 0, which is what > happens now. That means we'd have to do something like > > > Where the 'eglot-newline-and-indent-according-to-mode' has to calculate > the expected indentation. I don't understand how we'd expect the > formatters to do that indentation for us. They only care about code > already written, not code yet to be written. So if we'd have to > calculate that offset anyway, do we win much? Your "newline-and-indent" function could insert some bogus code (probably "bar;" or "bar();") and ask LSP to format that, then delete the bogus code. -- -- Stephe ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [ 2023-07-27 21:51 ` LSP vs Emacs indentation [ Stephen Leake @ 2023-07-28 5:28 ` Tassilo Horn 2023-07-28 13:29 ` Stefan Monnier 0 siblings, 1 reply; 8+ messages in thread From: Tassilo Horn @ 2023-07-28 5:28 UTC (permalink / raw) To: Stephen Leake Cc: João Távora, Theodor Thornhill, Eli Zaretskii, Stefan Monnier, emacs-devel Stephen Leake <stephen_leake@stephe-leake.org> writes: >> Where the 'eglot-newline-and-indent-according-to-mode' has to calculate >> the expected indentation. I don't understand how we'd expect the >> formatters to do that indentation for us. They only care about code >> already written, not code yet to be written. So if we'd have to >> calculate that offset anyway, do we win much? > > Your "newline-and-indent" function could insert some bogus code > (probably "bar;" or "bar();") and ask LSP to format that, then delete > the bogus code. I think the problem here is that this function would need to know at least some syntactically valid code to insert which depends on the language. Bye, Tassilo ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: LSP vs Emacs indentation [ 2023-07-28 5:28 ` Tassilo Horn @ 2023-07-28 13:29 ` Stefan Monnier 0 siblings, 0 replies; 8+ messages in thread From: Stefan Monnier @ 2023-07-28 13:29 UTC (permalink / raw) To: Tassilo Horn Cc: Stephen Leake, João Távora, Theodor Thornhill, Eli Zaretskii, emacs-devel >> Your "newline-and-indent" function could insert some bogus code >> (probably "bar;" or "bar();") and ask LSP to format that, then delete >> the bogus code. > > I think the problem here is that this function would need to know at > least some syntactically valid code to insert which depends on the > language. It would be easy to let major modes provide such a "syntactically valid dummy". For some languages it should be very easy (at least as easy as guessing an offset), but for others the valid dummy that needs to be used may depend on the context, and so may be just as hard as guessing an offset. In any case, leaving it empty may also result in "incorrect" reindentation of the rest. Stefan ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-07-28 13:29 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <87bkg4bkfu.fsf@fastmail.fm> [not found] ` <83a5voa328.fsf@gnu.org> [not found] ` <87h6pw9tpa.fsf@gmail.com> [not found] ` <875y6bm5ut.fsf@fastmail.fm> 2023-07-23 10:20 ` bug#64784: LSP vs Emacs indentation [Was: bug#64784: 30.0.50; Eglot: Lisp error: (wrong-type-argument number-or-marker-p return) in eglot--post-self-insert-hook] João Távora 2023-07-23 15:13 ` Theodor Thornhill 2023-07-23 16:42 ` João Távora 2023-07-24 17:03 ` Tassilo Horn 2023-07-24 18:13 ` Theodor Thornhill via Emacs development discussions. 2023-07-27 21:51 ` LSP vs Emacs indentation [ Stephen Leake 2023-07-28 5:28 ` Tassilo Horn 2023-07-28 13:29 ` Stefan Monnier
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).