unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Theodor Thornhill via "Emacs development discussions." <emacs-devel@gnu.org>
To: "Tassilo Horn" <tsdh@gnu.org>, "João Távora" <joaotavora@gmail.com>
Cc: Eli Zaretskii <eliz@gnu.org>,
	Stefan Monnier <monnier@iro.umontreal.ca>,
	emacs-devel@gnu.org
Subject: 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]
Date: Mon, 24 Jul 2023 20:13:04 +0200	[thread overview]
Message-ID: <871qgxxjnz.fsf@thornhill.no> (raw)
In-Reply-To: <87wmypb5dq.fsf@gnu.org>

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




  reply	other threads:[~2023-07-24 18:13 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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. [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=871qgxxjnz.fsf@thornhill.no \
    --to=emacs-devel@gnu.org \
    --cc=eliz@gnu.org \
    --cc=joaotavora@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    --cc=theo@thornhill.no \
    --cc=tsdh@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).