Add `prettier.forceFormatDocument` to code actions #2500

roottool · 2022-04-03T20:57:23Z

Run tests
Update the CHANGELOG.md with a summary of your changes

Resolves #1555

Overview

This PR adds two changes

Add prettier.forceFormatDocument to code actions
Adapt settings.json to this code action addition

Adapt `settings.json` to this code action addition

I reset editor.defaultFormatter and turn off editor.formatOnSave. Because this extension is able to format by source.prettier.forceFormatDocument in editor.codeActionsOnSave without the two items.

Run tests screenshot

feat: Add `forceFormatDocument` in code actions #1555 docs: Add a change by this PR #1555

wedi · 2022-04-14T21:37:44Z

@roottool Thank you for taking the initiative to get this long awaited feature in. 🎉

Is there a special reason why you chose to name the action source.prettier.forceFormatDocument? I am asking because it looks like there is a de facto standard to use fixAll.<productname>. This is used in Microsoft's own eslint extension and stylelint mentions in its docs that you can enable all auto fixing by setting "source.fixAll": true.

roottool · 2022-04-18T02:13:19Z

@wedi I forgot that I wrote the reason, sorry. It has two reason.

First, I wanted to be able to explicitly specify the order of execution. Because I don't know the execution order within "source.fixAll": true.
A codeaction that is added by this PR must be able to run from Prettier to AutoFix by ESLint in that order.

The above solution runs the default formatter (which can be set to Prettier) then uses the code action for eslint afterwards.
#1555

Second, I have decided that Source is more appropriate. Because I have determined that a statement ignoring the formatter's settings is not equivalent to a linter error.

Fix all actions automatically fix errors that have a clear fix that do not require user input.
VS Code API Docs - SourceFixAll

And this comment encouraged me to use Source.

For exposing actions for the CodeActionsOnSave option, the plugin needs to provide a Code Action by using the CodeAction API, and especially a CodeActionKind of type "source"
#1555 (comment)

FYI: This codeaction names were appropriated from the existing command name. Since this codeaction uses that the existing command.

prettier-vscode/package.nls.json

Line 3 in 991a53f

"ext.command.forceFormatDocument.title": "Format Document (Forced)",

If you have a good name for it, I'd be glad to hear it!

wedi · 2022-04-21T20:22:28Z

@roottool using the standard naming and choosing a custom execution order is not mutually exclusive. You can customize the order of execution by using the full names like this:

settings.json:

  "editor.codeActionsOnSave": [
    "source.organizeImports",
    "source.fixAll.eslint",
    "source.fixAll.stylelint"
  ],

As prettier fixes all source [code] styling issues, source.fixAll.prettier sounds reasonable to me but I am not further involved in this project. I just wanted to point out that there is a naming convention which you did not follow.

Personally, I'll be happy with whatever name gets chosen and I'm exited to see this merged. :)

Using `eslint-plugin-prettier` is slower than running prettier itself, the recommended approach is to run prettier first then follow by eslint to fix all the problems. VScode integration will be resolved once this PR gets merged prettier/prettier-vscode#2500.

ntotten · 2022-06-09T13:56:08Z

.vscode/settings.json

@@ -1,10 +1,10 @@
 // Place your settings in this file to overwrite default and user settings.
 {
  "editor.codeActionsOnSave": {
+    "source.prettier.forceFormatDocument": true,


Is this a new recommended best practice from the VS Code team or just preference?

No, it's not. I'm going to undo this difference if it is not necessary.

This difference has two intent.
First, The intent of this difference is to resolve #1277 by realizing the following quoted portion.

Originally posted by @rohit-gohri in #1277 (comment) is a working solution for running prettier before eslint.

The above solution runs the default formatter (which can be set to Prettier) then uses the code action for eslint afterwards.
#1555 (comment)

Second, it is for testing the created new code action in this PR.
The intention was to make it clear that I was testing through the code action.

It looks like Microsoft's recommended settings

Although you can also use the formatter on save using the setting editor.formatOnSave it is recommended to use the editor.codeActionsOnSave feature since it allows for better configurability.

https://github.com/microsoft/vscode-eslint

ntotten · 2022-06-09T13:57:20Z

src/CodeActionOnSave.ts

+const FORCE_FORMAT_DOCUMENT_CODE_ACTION_KIND = CodeActionKind.Source.append(
+  FORCE_FORMAT_DOCUMENT_COMMAND
+);
+const FORCE_FORMAT_DOCUMENT_TITLE = "Format Document (Forced)";


Why is it forced?

Because this code action uses a command of the same name that exists in the command palette.
But I would be willing to adopt any better title. 👍
For example,

Suggested change

const FORCE_FORMAT_DOCUMENT_TITLE = "Format Document (Forced)";

const FORMAT_DOCUMENT_TITLE = "Prettier: Format Document";

Could a code action be added for formatting? Such that if a user were to hit ctrl+shift+p in VS Code it would show something like "Prettier: Format Document"
#1555 (comment)

ntotten

A few comments

roottool · 2022-06-24T02:27:29Z

I replied to a few comments. But I'm sorry that it took me so long to reply.

This commit undo two things. - The previous incorrect commit - The `yarn prettier` execution by lint-staged This difference is not focused in this PR.

ntotten · 2022-07-21T17:42:50Z

I kind of missed the "forced" part of this PR. I guess I am confused what the point of this is? Why would you want to always force prettier to format files that you have set to be ignored?

nujhong · 2022-07-25T11:56:09Z

I think the contributor was re-using an existing command in the project as he mentioned https://github.com/prettier/prettier-vscode/blame/main/package.nls.json#L3

If I understood it correctly, the original commit dcc531f was aimed to provide a way to "Format Document (Forced)" which means 👉 format document specifically with formatter esbenp.prettier-vscod

Therefore, the goal of this PR was to re-use the same command so that it can be registered as a CodeAction and allowing users to specify the order of code actions by preference.

This ultimately solves the good old "Run Prettier First, then Run ESLint" problem as an alternative approach to doing it via CLI commands or eslint-plugin-prettier.

Looking forward to this being merged!

roottool · 2022-07-25T13:12:18Z

@nujhong Thank you for the explanation assistance!

@ntotten This CodeAction name includes "forced" is because I uniformed the name at an existing command name. The CodeAction re-uses "Format Document (Forced)" command already in this extension.

prettier-vscode/package.nls.json

Line 3 in b395990

"ext.command.forceFormatDocument.title": "Format Document (Forced)",

Including the appropriation of this name, the implementation intent is as he commented.
I have no intention of enforcing the format on me. I just appropriated the name.

nd0ut · 2022-12-28T12:55:49Z

Any updates?

ntotten · 2023-01-11T17:55:28Z

I am going to close this. This PR doesn't work and it doesn't have tests. If somebody is interested in contributing this please continue the discussion here: #1555

feat: Add prettier.forceFormatDocument to code actions #1555

83b9e1f

feat: Add `forceFormatDocument` in code actions #1555 docs: Add a change by this PR #1555

ntotten reviewed Jun 9, 2022

View reviewed changes

ntotten requested changes Jun 9, 2022

View reviewed changes

roottool added 3 commits June 27, 2022 01:35

Merge branch 'main' into feature/1555/add-code-action

9761dd2

style: Undo yarn prettier execution

45c383f

style: Undo yarn prettier execution

cc624e7

This commit undo two things. - The previous incorrect commit - The `yarn prettier` execution by lint-staged This difference is not focused in this PR.

ntotten closed this Jan 11, 2023

roottool deleted the feature/1555/add-code-action branch January 11, 2023 23:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `prettier.forceFormatDocument` to code actions #2500

Add `prettier.forceFormatDocument` to code actions #2500

roottool commented Apr 3, 2022

wedi commented Apr 14, 2022

roottool commented Apr 18, 2022

wedi commented Apr 21, 2022

ntotten Jun 9, 2022

roottool Jun 24, 2022

ya2s Oct 24, 2022

ntotten Jun 9, 2022

roottool Jun 24, 2022 •

edited

Loading

ntotten left a comment

roottool commented Jun 24, 2022

ntotten commented Jul 21, 2022

nujhong commented Jul 25, 2022

roottool commented Jul 25, 2022

nd0ut commented Dec 28, 2022

ntotten commented Jan 11, 2023

	const FORCE_FORMAT_DOCUMENT_TITLE = "Format Document (Forced)";
	const FORMAT_DOCUMENT_TITLE = "Prettier: Format Document";

Add prettier.forceFormatDocument to code actions #2500

Add prettier.forceFormatDocument to code actions #2500

Conversation

roottool commented Apr 3, 2022

Overview

Adapt settings.json to this code action addition

Run tests screenshot

wedi commented Apr 14, 2022

roottool commented Apr 18, 2022

wedi commented Apr 21, 2022

ntotten Jun 9, 2022

Choose a reason for hiding this comment

roottool Jun 24, 2022

Choose a reason for hiding this comment

ya2s Oct 24, 2022

Choose a reason for hiding this comment

ntotten Jun 9, 2022

Choose a reason for hiding this comment

roottool Jun 24, 2022 • edited Loading

Choose a reason for hiding this comment

ntotten left a comment

Choose a reason for hiding this comment

roottool commented Jun 24, 2022

ntotten commented Jul 21, 2022

nujhong commented Jul 25, 2022

roottool commented Jul 25, 2022

nd0ut commented Dec 28, 2022

ntotten commented Jan 11, 2023

Add `prettier.forceFormatDocument` to code actions #2500

Add `prettier.forceFormatDocument` to code actions #2500

Adapt `settings.json` to this code action addition

roottool Jun 24, 2022 •

edited

Loading