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]

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]

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

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]

[Theodor Thornhill]

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

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]

[João Távora]

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

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]

[Tassilo Horn]

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

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]

[Theodor Thornhill via Emacs development discussions.]

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

Re: LSP vs Emacs indentation [

[Stephen Leake]

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

Re: LSP vs Emacs indentation [

[Tassilo Horn]

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

Re: LSP vs Emacs indentation [

[Stefan Monnier]

>> 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