Close paren on type formatting reformats whole document instead of current expression about racket-langserver HOT 16 CLOSED

Oh, it's a bug. I thought it's a feature that it reformat the whole document.

from racket-langserver.

After some search, I found only a little editor supports onTypeFormatting:

• vscode - cursor position, code here and getPosition
• neovim - no support
• helix - no support
• emacs lsp mode - cursor position, code here

I think they all looked into vscode's implementation. So a possible solution is to assume it's cursor position.

from racket-langserver.

Agree

from racket-langserver.

Sounds like a good approach to me, thanks for working on this. 😄

from racket-langserver.

Interesting, in doc.rkt:190-208 should already make a filter

from racket-langserver.

If there can have a bug, it should be doc-find-containing-paren

from racket-langserver.

I think I know where the bug come from.
From specification - DocumentOnTypeFormattingParams:

/**
 * The position around which the on type formatting should happen.
 * This is not necessarily the exact position where the character denoted
 * by the property `ch` got typed.
 */
position: Position;

The position could not be the position of the inserted character. It's an actually useless parameter.
But in code

It uses pos to find the innermost pair of parentheses that contains the position pos. It can be wrong as the reason above.

In the example code, the position that the client passed is this:

(publisher))
            ^

Our code should not depend on the precise definition of position.

from racket-langserver.

Not quite? It says the position “not necessary” be the same, but it still is the same in most cases, where you point out the position is after the inserted, I think that’s correct.

Maybe we can use pos-1 instead of pos?

Another solution should be we stop using find parentheses, but the find prev sexp and find next sexp to detect the scope?

from racket-langserver.

No. The current code expects position is here:

(publisher))
           ^

This is the position of ch in the request.

Suppose position can be position-of(ch) or position-of(ch) + 1, then we can't process this case:

)))
 ^
 ch

It would be ambiguous. This is a problem.

from racket-langserver.

Yes, but I think a finite scope is still better than the whole file.

from racket-langserver.

I have an evil idea. We can save the position information in did-change!, and use it in on-type-formatting. So we can do precise formatting.
Another normal idea is, because we only format from line x to line y currently instead of character level, we can find the last closed parenthese at this line, and find where its matching open parenthese at.

from racket-langserver.

The second idea can have a condition, that if text[position] has an closed parenthese, then use doc-find-containing-paren.

from racket-langserver.

I prefer the evil solution. We just need to add a last-closed-paren field in struct Doc, and maintain it.

from racket-langserver.

I was curious what other lang servers do for this case...

Looking at Rust Analyzer, it seems like they assume that for on-type formatting, you can find a precise position by subtracting one character from the position in the protocol message. It seems to work for this one example here... but of course, I'm not sure what other clients might do.

If we really can't depend on the position field, then perhaps one of @6cdh's ideas is they way to go.

from racket-langserver.

Rust analyzer assume there has no other plugin will affect the position

from racket-langserver.

It's been 5 days since our last discussion. What are your thoughts? I think the solution that assuming it's the cursor position looks like a safe option. If you agree, I can do this today. It would be a small fix.

from racket-langserver.