Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird instability in qmd #77

Open
DavisVaughan opened this issue Nov 27, 2024 · 4 comments
Open

Weird instability in qmd #77

DavisVaughan opened this issue Nov 27, 2024 · 4 comments

Comments

@DavisVaughan
Copy link
Collaborator

DavisVaughan commented Nov 27, 2024

---
title: "Untitled"
format: html
---

```{r}
df |>
  mutate(x = foo()) |>
  mutate(y = bar()) |>
  summarise()
```

It tries to flatten, looks like it succeeds (and it should), but then you save again and it re-expands? It's probably worth trying to look at the intermediate vdocs here.

Screen.Recording.2024-11-27.at.5.25.16.PM.mov

We get this on the 2nd vdoc, which looks like it formats "stable-y" in a separate R file on its own, so im not sure whats up

#
#
#
#
#
#
df |> mutate(x = foo()) |> mutate(y = bar()) |> summarise()
#
#
#

Exact text is

"#\n#\n#\n#\n#\n#\ndf |> mutate(x = foo()) |> mutate(y = bar()) |> summarise()\n#\n#\n#\n"
@DavisVaughan
Copy link
Collaborator Author

Interestingly a single "save" calls us twice, but neither calls produce text edits 🤔

This is what i get from hitting "save" when this was in the qmd cell

df |> mutate(x = foo()) |> mutate(y = bar()) |> summarise()
[Info  - 5:33:08 PM] Incoming: DidOpenTextDocument(
    DidOpenTextDocumentParams {
        text_document: TextDocumentItem {
            uri: Url {
                scheme: "file",
                cannot_be_a_base: false,
                username: "",
                password: None,
                host: None,
                port: None,
                path: "/Users/davis/Desktop/.vdoc.r",
                query: None,
                fragment: None,
            },
            language_id: "r",
            version: 1,
            text: "#\n#\n#\n#\n#\n#\ndf |> mutate(x = foo()) |> mutate(y = bar()) |> summarise()\n#\n#\n#\n",
        },
    },
)
[Info  - 5:33:08 PM] Incoming: DocumentFormatting(
    DocumentFormattingParams {
        text_document: TextDocumentIdentifier {
            uri: Url {
                scheme: "file",
                cannot_be_a_base: false,
                username: "",
                password: None,
                host: None,
                port: None,
                path: "/Users/davis/Desktop/.vdoc.r",
                query: None,
                fragment: None,
            },
        },
        options: FormattingOptions {
            tab_size: 2,
            insert_spaces: true,
            properties: {},
            trim_trailing_whitespace: Some(
                true,
            ),
            insert_final_newline: None,
            trim_final_newlines: Some(
                true,
            ),
        },
        work_done_progress_params: WorkDoneProgressParams {
            work_done_token: None,
        },
    },
)
[Info  - 5:33:08 PM] Outgoing DocumentFormatting(
    Some(
        [],
    ),
)
[Info  - 5:33:08 PM] Incoming: DocumentFormatting(
    DocumentFormattingParams {
        text_document: TextDocumentIdentifier {
            uri: Url {
                scheme: "file",
                cannot_be_a_base: false,
                username: "",
                password: None,
                host: None,
                port: None,
                path: "/Users/davis/Desktop/.vdoc.r",
                query: None,
                fragment: None,
            },
        },
        options: FormattingOptions {
            tab_size: 2,
            insert_spaces: true,
            properties: {},
            trim_trailing_whitespace: Some(
                true,
            ),
            insert_final_newline: None,
            trim_final_newlines: Some(
                true,
            ),
        },
        work_done_progress_params: WorkDoneProgressParams {
            work_done_token: None,
        },
    },
)
[Info  - 5:33:08 PM] Outgoing DocumentFormatting(
    Some(
        [],
    ),
)
[Info  - 5:33:08 PM] Incoming: DidCloseTextDocument(
    DidCloseTextDocumentParams {
        text_document: TextDocumentIdentifier {
            uri: Url {
                scheme: "file",
                cannot_be_a_base: false,
                username: "",
                password: None,
                host: None,
                port: None,
                path: "/Users/davis/Desktop/.vdoc.r",
                query: None,
                fragment: None,
            },
        },
    },
)
[Info  - 5:33:08 PM] did_close(): closed document with URI: 'file:///Users/davis/Desktop/.vdoc.r'.

@DavisVaughan
Copy link
Collaborator Author

Oh I think Quarto may have an R formatter getting in the way here? If you turn off air you still get the expanded pipeline when you start flat.

@DavisVaughan
Copy link
Collaborator Author

Oh I think it only happens in positron and is because styler is trying to get called!

@DavisVaughan
Copy link
Collaborator Author

DavisVaughan commented Dec 2, 2024

Ugh ok gross I think I've figured this out, quarto calls:

https://github.com/posit-dev/positron/blob/1492e58b74678103a84810ae453cfa65cc4662ad/src/vs/editor/contrib/format/browser/format.ts#L399-L415

And for some crazy reason, that getDocumentFormattingEditsUntilResult() will keep calling providers until one returns a non-empty array 😢 . So in the above example when the line is fully "flat" and we hit save:

  • air is called, returns "no edits"
  • but then positron-r is called! And styler thinks the line should be formatted across multiple lines

Related microsoft/vscode#212124 where it affected black and ruff interacting!

It looks like the solution for notebooks was to change to the new getDocumentFormattingEditsWithSelectedProvider() API, we should probably do that in quarto too!

microsoft/vscode#212185

Ugh but getDocumentFormattingEditsWithSelectedProvider() isn't exposed to extensions like getDocumentFormattingEditsUntilResult() is. I think our path forward is:

  • Ask VS Code to either:
    • Expose getDocumentFormattingEditsWithSelectedProvider() in a companion to vscode.executeFormatDocumentProvider
    • OR change vscode.executeFormatDocumentProvider to use that instead of getDocumentFormattingEditsUntilResult()
  • In the meantime, turn off positron-r's registration of styler as a format document provider, so there is only 1, air.

Turning off positron-r's styler registration is the long term plan anyways, so VS Code support isn't critical, but quarto should probably use this new api anyways because cycling through all formatters is a bad idea in a notebook

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant