provideDocumentFormattingEdits called twice (formatting applied twice)

[zqstack]

Somehow when adding my custom formatter extension for TypeScript to Multiple Formatters, it seems to be loaded twice.

provideDocumentFormattingEdits is called twice as I console.log(), while activate() is called only once.

I wonder why this is happening. It does not happen when loaded directly as default formatter instead of using Multiple Formatters. Even in the most stripped-down version below.

settings.json (workspace - user does not have this setting)

"[typescript]": {
    "editor.defaultFormatter": "jota0222.multi-formatter",
    "multiFormatter.formatterList": [
        "vscode.typescript-language-features",
        "undefined_publisher.my-custom-formatter"
    ],
},

extension.ts

export function activate(context: vscode.ExtensionContext) {

	console.log('activate');

      vscode.languages.registerDocumentFormattingEditProvider(
          [ 'typescript', 'javascript' ]
      , {
            provideDocumentFormattingEdits(document: vscode.TextDocument): vscode.TextEdit[] {
                  console.log('provideDocumentFormattingEdits:');
      
                  return [];
            }
      });
}

package.json (extension - parts which may be relevant?)

"engines": {
  "vscode": "^1.86.0"
},
"categories": [
  "Formatters"
],
"activationEvents": [
  "onLanguage:typescript"
],
"contributes": {
},

[jota0222]

Hi @zqstack. Sorrry for de delayed reply.

Some weeks ago, a fix for formatters running more than once due to infinite loop of savings was added to de codebase, which happened, apparently, because the formatters were saving by themselves after format. Normally VSCode will handle the saving when a formatter runs after save if the format after save configuration is enabled.

In the code you provided you don't seem to be saving but, Did you manage to try with the last version of the extension? It should prevent the formatters to run twice, otherwise, I don't see what can be the issue, so I will ask you to provide more information, like if you have the format on save enabled in your configuration or if configured VSCode to format each time you do a change no matter what it is.

[a-laughlin]

I'm getting the same result. Wondering if this line is necessary since formatOnSave is already a "save" action that triggers the formatter. I'll clone the repo and take a closer look.

[a-laughlin]

Created a fresh extension and tried running it. Calling commands.executeCommand("workbench.action.files.save") seems to cause more problems than it solves. Calling commands.executeCommand("editor.action.formatDocument") with an additional formatter still saves the file without the additional save action.

The save seems to cause an additional run. If you execute a save when there are formatting changes, the extension runs twice. If you execute a save when there are no changes, it only runs once.

[jota0222]

Thanks @a-laughlin. Lamentably, it seems that what you say isn't always the case, see the issue #2, that's why that line was added in the first place.

Edit: Anyway, if there is a bug, is probably in these changes, since, as you can see in line 71 to 76 and 85 to 86, we already try to prevent to run the formatters twice even if format on save is enabled.

[jota0222]

I have added a commit that probably solves your issue. Could you test the last vesion of the app?

[a-laughlin]

Thanks for the update @jota0222. I also have some questions after testing MultiFormatter's core approach in a fresh plugin.

Lamentably, it seems that what you say isn't always the case, see the issue #2, that's why that line was added in the first place.

Anyway, if there is a bug, is probably in these changes, since, as you can see in line 71 to 76 and 85 to 86, we already try to prevent to run the formatters twice even if format on save is enabled.

How sure are you that MultiFormatter caused the problem in #2? I can't reproduce that issue when I write a formatter plugin from scratch that runs multiple formatters in the same way. Nor can I find any instance of saving the document when I search vscode-prettier. My guess is that the cause of #2 was another plugin, not MultiFormatter.

I think adding workbench.action.files.save... was a solution to that other plugin's bug. The fix of duplicating formatOnSave's functionality in MultiFormatter created the double-saving problem that those changes had to solve. In other words, both saving and all the code to prevent formatting twice can be safely removed.

Are you able to reproduce the problem of not saving when formatOnSave is set and you remove workbench.action.files.save... line?

[jota0222]

Yes @a-laughlin, I was able to reproduce the issue when it was posted. Probably you are right and VSCode has received an update in which this doesn't happen anymore, however, I still think it's necessary to leave the save command there for older versions to work as expected.

Also, as you may noticed, the issue was posted by someone and other person was who "solved" the issue, that suggests that it was happening to different people.

Let me know if you still face the issue after the last update posted to see what can we do next.

PS: This is not something I could confirm in the moment, but I think the issue was caused by the way the formatters are executed by the plugin, it changes the default formatter and runs the formatSelection command next, which doesn't involve formatOnSave feature because we are just not saving, and also there is no way to wait until the external formatters finish to format. So, I guess the multi-formatter was finishing before the other formatters, causing changes to be left not saved

[a-laughlin]

saveWithoutFormatting does fix dual running issue. Thanks for adding that.

and also there is no way to wait until the external formatters finish to format. So, I guess the multi-formatter was finishing before the other formatters, causing changes to be left not saved

Interesting race condition case. VSCode not waiting for a formatter to finish seems like it would cause many plugin interaction problems for the VSCode team. My guess is that they'd have to wait somewhere.

This is not something I could confirm in the moment, but I think the issue was caused by the way the formatters are executed by the plugin, it changes the default formatter and runs the formatSelection command next, which doesn't involve formatOnSave feature because we are just not saving

I just did a test that will hopefully help. It uses a separate plugin that only logs saves:

context.subscriptions.push(
    workspace.onDidSaveTextDocument((document)=>{
      if(!document.fileName.endsWith('foo.ts')) {return;}
      logger.appendLine(`onDidSaveTextDocument (${document.fileName}) ~ document.isDirty:${document.isDirty}.`);
    }),
    workspace.onWillSaveTextDocument((e)=>{
      if(!e.document.fileName.endsWith('foo.ts')) {return;}
      logger.appendLine(`onWillSaveTextDocument (${e.document.fileName})  ~ document.isDirty:${e.document.isDirty}`);
    })
);

with no workspace settings, and these user settings:

{
  formatOnSave:true,
  "[typescript]":{
    "editor.defaultFormatter": "Jota0222.multi-formatter",
    "multiFormatter.formatterList": [
      "esbenp.prettier-vscode"
    ]
  }
}

Results of saving the document:

// Prettier defaultFormatter. Prettier made no changes
onWillSaveTextDocument (foo.ts)  ~ document.isDirty:false
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

// Prettier defaultFormatter. Prettier made changes
onWillSaveTextDocument (foo.ts)  ~ document.isDirty:true
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

// MultiFormatter defaultFormatter. Prettier made changes
onWillSaveTextDocument (foo.ts)  ~ document.isDirty:true
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

// MultiFormatter defaultFormatter. Prettier made no changes
onWillSaveTextDocument (foo.ts)  ~ document.isDirty:false
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

Results of "format selection" without saving:

// Prettier defaultFormatter. Prettier made no changes
... nothing logged. Document keeps state "saved" in tab indicator ...

// Prettier defaultFormatter. Prettier made changes
... nothing logged. Document gains state "unsaved" in tab indicator ...

// MultiFormatter defaultFormatter. Prettier made no changes
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

// MultiFormatter defaultFormatter. Prettier made changes
onWillSaveTextDocument (foo.ts)  ~ document.isDirty:true
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.
onDidSaveTextDocument (foo.ts) ~ document.isDirty:false.

It appears that MultiFormatter saves the file even when it shouldn't. It's possible to insert a check like for document.isDirty, but that will still run the save when trying to format without saving. Cases to handle start to get complicated, and they expose the plugin to more bugs coming from changes in the VSCode extension API.

I get your desire for the plugin to work with older versions. At the same time, the effort of supporting multiple versions ends up being a lot, while introducing vectors for more bugs. You're welcome to do that if you want. Personally, I'd save the effort by telling folks with non-saving issues that they need to update their vscode or other formatters.

[jota0222]

@a-laughlin Can you tell me if the tests you did were using the version with the saveWithoutFormatting? 'cause I'm testing here with isDirty and it's always dirty when it gets back to the multi-formatter after using Prettier, so, at the end, the isDirty validation would be useless and unnecessary. Would it be prettier who is doing its saves just after we do ours? If that's the case, there is little we can do.