prettier / prettier-vscode

Visual Studio Code extension for Prettier
https://marketplace.visualstudio.com/items?itemName=esbenp.prettier-vscode
MIT License
5.04k stars 446 forks source link

Use VSCode tab/spacing settings instead of prettier specific settings #1515

Closed JVimes closed 3 years ago

JVimes commented 3 years ago

Summary

For GraphQL, the formatter seems to indent by 2 spaces even when VSCode is configured to indent by a different number, or by tabs. This behavior may be specific to GraphQL, I don't see it with JSON.

Steps To Reproduce:

  1. Open a GraphQL file, such as query.graphql:

    query {
      foo {
        bar
      }
    }
  2. Try different indentation settings in VSCode, and run "format document" for each.

Expected result

Formatting uses VSCode's selected indentation: tabs, spaces, number of spaces.

image

Actual result

Indentation is always done with 2 spaces.

Additional information

VS Code Version: 1.49.0-insider

Prettier Extension Version: v5.1.3

OS and version: Version 2004 (OS Build 19041.450)

Prettier Log Output

["INFO" - 9:33:15 AM] Formatting c:\path\to\query.graphql
["INFO" - 9:33:15 AM] Using ignore file (if present) at undefined
["INFO" - 9:33:15 AM] Using bundled version of prettier.
["INFO" - 9:33:15 AM] File Info:
{
  "ignored": false,
  "inferredParser": "graphql"
}
["INFO" - 9:33:15 AM] No local configuration (i.e. .prettierrc or .editorconfig) detected, falling back to VS Code configuration
["INFO" - 9:33:15 AM] Prettier Options:
{
  "arrowParens": "always",
  "bracketSpacing": true,
  "endOfLine": "lf",
  "htmlWhitespaceSensitivity": "css",
  "insertPragma": false,
  "jsxBracketSameLine": false,
  "jsxSingleQuote": false,
  "printWidth": 80,
  "proseWrap": "preserve",
  "quoteProps": "as-needed",
  "requirePragma": false,
  "semi": true,
  "singleQuote": false,
  "tabWidth": 2,
  "trailingComma": "es5",
  "useTabs": false,
  "vueIndentScriptAndStyle": false,
  "filepath": "c:\\path\\to\\query.graphql",
  "parser": "graphql"
}
["INFO" - 9:33:15 AM] Formatting completed in 33.6648ms.
ntotten commented 3 years ago

That setting is for the built-in formatter, not prettier. Please see: https://github.com/prettier/prettier-vscode#configuration

JVimes commented 3 years ago

@ntotten, I think maybe you misunderstood. Formatters should all use the IDE's indentation setting, whether they ship with the product or are installed with an extension. This ticket is asking prettier-vscode to behave like users expect. Is there a reason not to?

ntotten commented 3 years ago

After looking into this, it's not possible to handle this case. What we would do is fallback to use the VS Code options if the prettier config options are not set. However, due to the VS Code config API we can't actually tell if the use has set prettier.useTabs or not. The only way to implement this would be to remove our custom config options prettier.tabWidth and prettier.useTabs and only use the built-in options. However, as this would be a breaking change it's not something I think is worth it. Plus there also could be cases where users actually want them separate. For example, if they have one set of options when using the prettier formatter and another set of options when using the built-in formatters.

JVimes commented 3 years ago

Thanks for looking into it, @ntotten. I tend to think that "extension-specific indentation" is a false use case, and is harmful to the experience of practically every user. Was it actually a design decision, or did it just happen incidentally as the extension was written? Please reconsider, especially if anyone else expresses or has expressed trouble getting indentation working. Hopefully there will someday be the API to support fallback. Thanks again for investigating.

ntotten commented 3 years ago

Let me leave this open to gather comments. I'm open to consider it, but it will take some time as we'll have to introduce a breaking change.

kaptcha0 commented 3 years ago

I have also noticed this with other languages. No matter how many times I change it in VSCode settings, it never makes a difference.

yellowjacketcoder commented 3 years ago

I think an alternate solution would be to integrate Prettier's prettier.useTabs and prettier.tabWidth into VS Code's status bar using a custom StatusBarItem of VS Code Extension API with the ability to change it just like VS Code's own setting.

JVimes commented 3 years ago

Why have two indentation settings, one unused, even in the status bar? I think extension-specific indentation is not a real use case, even in the status bar.

JVimes commented 3 years ago

I'm not sure this ticket exactly qualifies as a breaking change, since it fixes the expected UX at the expense if a non-feature, I would argue. I think most users will just notice that "indentation starts working".

thorn0 commented 3 years ago

Prettier's settings should have priority to make sure that formatting results are consistent across different ways to run Prettier (CLI, extensions for different editors). This is to guarantee that everybody in the team gets the same formatting results.

JVimes commented 3 years ago

I don't agree. In an editor, I expect the indentation setting to work. I don't expect CLI behavior.

JVimes commented 3 years ago

To clarify, it should be the same on CLI, but by using something non-extension-specific, like editorconfig files. Not by implementing a redundant setting.

thorn0 commented 3 years ago

@JVimes Prettier's priority is to make sure that

everybody in the team gets the same formatting results

What you want directly contradicts this goal. Maybe Prettier just isn't the right tool for you.

JVimes commented 3 years ago

@thorn0 No, .editorconfig ensures team-wide formatting and is supported by a plethora of editors and tools, so it supports your goal better than Prettier's setting does.

JVimes commented 3 years ago

@thorn0 Let me put it a different way: Prettier's job is to format code, not to enforce that across the team. .editorconfig's job is to enforce that formatting across the team. And it's not okay to break part of VSCode's main UI in order for Prettier Extension to overreach its purpose.

thorn0 commented 3 years ago

editorconfig ensures team-wide formatting and is supported by a plethora of editors and tools, so it supports your goal better than Prettier's setting does.

In a way, they're the same as Prettier reads settings from .editorconfig.

Prettier's job is to format code, not to enforce that across the team.

As one of the maintainers, I guess I'm more familiar with the project's goals than you.

thorn0 commented 3 years ago

@ntotten

After looking into this, it's not possible to handle this case. What we would do is fallback to use the VS Code options if the prettier config options are not set.

Prettier options are always set. When they're not set explicitly, it's important that their default values are used. So such a fallback is indeed impossible.

JVimes commented 3 years ago

[@thorn0]: As one of the maintainers, I guess I'm more familiar with the project's goals than you.

Maintainer's don't decide user goals. The community does not accept breaking VSCode's main UI as a "goal".

Prettier should read VSCode's setting and VSCode should handle .editorconfig.

thorn0 commented 3 years ago

It should be handled the same way it's handled in the EditorConfig extension.

DanAlexson90 commented 3 years ago

@thorn0 No, .editorconfig ensures team-wide formatting and is supported by a plethora of editors and tools, so it supports your goal better than Prettier's setting does.

Prettier extension shouldn't rely on the presence of the EditorConfig extension.

DanAlexson90 commented 3 years ago

Prettier should read VSCode's setting and VSCode should handle .editorconfig.

VS Code doesn't handle the .editorconfig file, it is the EditorConfig extension that does.

monteiz commented 3 years ago

Imagine a team agreeing on using a certain Prettier configuration, but some members using different vscode settings, and those settings being in force. It would be a mess.

If a user/team decides to use Prettier, he/she/it naturally expects all Prettier settings overriding vscode equivalent settings.

This is the natural expected behaviour as well as the guarantee that code format is consistent among team members.

ntotten commented 3 years ago

@monteiz This is how the extension behaves. If you include a .prettierrc or any other configuration in your project those settings ALWAY are used and no VS Code settings will be used. The VS Code settings are ONLY used when there is no project configuration. The advice of the prettier team is that EVERY project should have a .prettierrc file even if it uses the default settings, in which case the file should only include{}. This ensures that no matter what, every user of the project has the same settings and will have the same formatting output regardless of how they run prettier - through VS Code or through the CLI.

monteiz commented 3 years ago

@ntotten I agree on the .prettierrc having the absolute priority. But then the overall natural priority order should be:

.prettierrc > Prettier settings > VS Code settings

Because at the moment I am having unexpected behaviours (as described in #1856).

ntotten commented 3 years ago

Unfortunately, from what I can tell, the VS Code Extension API does not allow the Prettier settings > VS Code settings to be implemented. There isn't a way for the extension to tell if the tabs/spaces settings passed to the formatter are set by the user or just the default. We either have to use settings like prettier.useTabs OR the VS Code tab/spaces settings.

monteiz commented 3 years ago

@ntotten I did not dig into the code, but it seems possible, at least for the User settings (which makes me think it is possible also for the Workspace settings).

As described in #1856, the prettier.useTabs setting works correctly (overriding the VS Code settings), but only if defined in the User settings. It is ignored in the Workspace settings, but that makes me think more of a bug of the prettier-vscode extension.

thorn0 commented 3 years ago

VS Code Extension API does not allow the Prettier settings > VS Code settings

The EditorConfig extension somehow does exactly that.

ntotten commented 3 years ago

I'll have to dig in, but it might be because of how they have their defaults set. We default VSCode settings to the Prettier defaults. I.e. tabSize: 2. If we defaulted to null we could tell if it was set, but as it is now, we can't tell if it is set to 2 or just defaulted to 2.

ntotten commented 3 years ago

@monteiz i will reopen that bug. I didn't realize what you were saying. That being said, I don't actually think that is true. We don't distinguish between user and workspace settings. VSCode simply provides the resolved value. You'll need to provide a repro.

ntotten commented 3 years ago

@thorn0 What extension are you referring? I found this one, but it doesn't provide any extension specific settings like this. https://github.com/editorconfig/editorconfig-vscode/blob/master/package.json#L45

thorn0 commented 3 years ago

Yes, that one. It reads settings from .editorconfig files and those settings override VS Code's settings.

E.g. if you have an .editorconfig file like this:

[*]
indent_size = 3

and .vscode/settings.json with this setting:

  "editor.tabSize": 4

then the value 3 has priority. In particular, it's shown in the status bar.

ntotten commented 3 years ago

Yes, this extension does this and has done that for a long time.

The extension DOES fall back to use the Prettier provided VSCode settings. So if you set prettier.tabSize for example, that setting will be used UNLESS an Prettier config file is found.

What this issue is asking for is that the settings ALSO fall back to the VSCode native tab/spaces setting. We currently DO NOT do this because it isn't straight forward.

When a formatter is run, VS Code passes a tabs/spaces settings for the editor. The key is VSCode ALWAYS passes the built-in tabs/spaces settings. They always have a value. So we only have two choices:

  1. Use Prettier extension settings prettier.useTabsand prettier.tabWidth settings unless a prettier config file is found. This is how the extension behaves today.

  2. Use the built-in VSCode tab/spaces settings unless a prettier config file is found. If we did this we would deprecate the Prettier extension settings prettier.useTabsand prettier.tabWidth.

thorn0 commented 3 years ago

Yes, this extension does this and has done that for a long time.

I'm really not sure about that. Just tested it again. But turns out it's a totally separate issue → #1859

What this issue is asking for is that the settings ALSO fall back to the VSCode native tab/spaces setting. We currently DO NOT do this because it isn't straight forward.

This should be a wontfix IMHO. The current behavior is a good trade-off.

ntotten commented 3 years ago

Ah, yeah. The issue you opened is a whole different thing. I'm not sure we can change that either, but I'll move conversation there.

ntotten commented 3 years ago

The vscode tab issue is now at #1859. The behavior discussed in this original issue seems to be working as expected per my above comment: https://github.com/prettier/prettier-vscode/issues/1515#issuecomment-802304888

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.