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

Consider not setting Air as the editor.defaultFormatter #162

Open
DavisVaughan opened this issue Jan 15, 2025 · 2 comments
Open

Consider not setting Air as the editor.defaultFormatter #162

DavisVaughan opened this issue Jan 15, 2025 · 2 comments

Comments

@DavisVaughan
Copy link
Collaborator

This is a user setting

"editor.defaultFormatter": "Posit.air"

@DavisVaughan
Copy link
Collaborator Author

We were going to do this in #171 but there are two main reasons we said no right now:

  • Positron currently ships a formatter that uses styler (it does not mark it as "default"). This formatter "competes" with ours if we don't set ourselves as the default. You get a silent no-op if you just use Format on Save, and you get prompted to choose a default if you explicitly call Format Document. This was likely the original reason we set editor.defaultFormatter. The right thing to do is to eventually remove the Positron formatter.
Image

https://github.com/posit-dev/positron/blob/2d4492c35638ea01e611dec599e9d6bbd60e9df8/extensions/positron-r/src/formatting.ts#L31

  • @lionel- thinks that we may want to unconditionally keep this setting, regardless of what Positron sets. The reasoning was that Air is positioned to be a full language server for R eventually (not just a formatter), and it should be in charge of all the language smarts for R, including setting itself as the default formatter. I tend to disagree, and think that this is a user setting that we should not override. Practically, if a user installed another R formatter extension alongside Air, the user would not get the "multiple formatter" notification that is intended to show up, because we are already positioned as the default. I think that would be a confusing experience (sure, the user can set editor.defaultFormatter in their user settings to switch to the other one, but they aren't told they need to because they don't get this notification). Notably none of rust-analyzer, the Microsoft C++ extension, clangd, or ruff (all language servers that also format, with some doing more than others) set themselves as the default formatter. Instead they typically provide documentation on how to set editor.defaultFormatter and why you might want to, but that's it. I do not actually think there is much upside to setting yourself as the default formatter (besides the current Positron issue), only some potential confusion if another formatter is installed alongside Air.

@DavisVaughan DavisVaughan changed the title Don't set ourselves as the default formatter Don't set Air as the editor.defaultFormatter Jan 16, 2025
@lionel-
Copy link
Collaborator

lionel- commented Jan 17, 2025

My main thought is that a formatter isn't that different from other language features a language extension provides unconditionally for a language, especially when it offers format-as-you-type to fix up indentation (as we will in the future). Having a formatter mixup in that case makes for a confusing experience. And having to set it manually, while it's certainly something that professional developers are accustomed to, is not good user experience for our target audience.

Alternatively we could set the default formatter in positron-r to Air and leave VS Code users on their own for configuration. I think it's a good default though, even if unconventional in the vscode landscape (which I agree is a good argument not to do it).

@lionel- lionel- changed the title Don't set Air as the editor.defaultFormatter Consider not setting Air as the editor.defaultFormatter Jan 17, 2025
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

Successfully merging a pull request may close this issue.

2 participants