fix #3338 Format Document on save

[lmcbout]

Fix #3338
This is a follow-up of the pr #3640
Since the " [monaco] re-implementation of fireWillSaveModel " on Jan 2nd, it is possible to use the "format on Save" .

Format on Save is only available when the preference autoSave is "OFF"
and the preference 'editor.formatOnSave' is "true"

Signed-off-by: Jacques Bouthillier jacques.bouthillier@ericsson.com

[paul-marechal]

event.reason !== TextDocumentSaveReason.AfterDelay

[lmcbout]

Done for the next version, will use event.reason === TextDocumentSaveReason.Manual

[akosyakov]

it should be reverted right? save should not do anything if a document is not dirty

[akosyakov]

it should be reverted? save should be aborted if a document is not dirty

[elaihau]

I guess what @lmcbout wanted to do here is "formatting the file on users hitting CTRL+S even if the file is not dirty" - vsCode does it.

[lmcbout]

This is exactly the reason, to allow "CTRL+S" like vscode does

[akosyakov]

Please check whether VS Code actually save changes or only try to apply formatting. In cloud case sending unchanged content is expensive and we should avoid doing it.

[lmcbout]

I tried VSCode with the file (theia/.vscode/launch.json). The file is not dirty. By selecting "CTRL+S", it reformatted the file and saved it.

[akosyakov]

I meant in our case if formatting does nothing we should not write anything to the disk. It is expensive for large files.

[lmcbout]

After the formatting, the frontend is using the monaco-editor-model.doSave()
In that method, we check if there is some changes (~ line 320) code:

        const changes = this.popContentChanges();
        if (changes.length === 0) {
            return;
        }

This is the same as testing isDirty(). In fact we could use:

if (this.dirty) return;

to replace those lines above and it would do the same here . So as a resume, the file remains in the frontend if there is no formatting , they are not sent to the backend when there is no changes.

[vince-fugnitto]

I noticed the command Save All does not work properly.

Say for example I have three .json files with auto-save off.
If I attempt to Save All, all three files are saved, but only the last file in the editor actually gets formatted.

[lmcbout]

Question:
In this PR, in monaco-editor-providers.createMonacoEditor(), I try to use the action from the editor and use the"Save All" command from the menu, it saves and re-format all files even the one not having the focus, but after that, if I modify the file having the focus, it starts to add the new data in the wrong editor. I can see it by the dirty icon showing on the wrong file.
From this PR , just replace the following line:

await this.commandService.executeCommand('monaco.editor.action.formatDocument');
by
await editor.runAction('editor.action.formatDocument');

Question: is the editor.runAction() broken ?
Note:
Set the pref: "editor.formatOnSave": true
the following video: Started from two files to be re-formatted when selecting "Save All" and after the save , try to enter new data in the file having the focus, but the dirty flag goes to the other file and the new data as well.

[lmcbout]

Tested file: ".java, .cpp, .ts, .json"

Note: need to set the preference to format the file: "editor.formatOnSave": true
In monaco-editor-provider.ts
modify method createMonacoEditor(),

if we use the command:

      await this.commandServiceFactory().executeCommand('monaco.editor.action.formatDocument');

it works fine except if we select from the menu "Save All", it saves all files but only format the file having the focus. The other files are just saved.

if we use the command:

      await editor.runAction('editor.action.formatDocument');

works fine with one file. With multiple file to save (Save All), it re-formats all files and saves them, but after, if you type new code , it will end-up in a file not necessary having the focus. (Is an issue if we try to use this command)

---

To Test:

Preparation : step 1 to 5

1. set the preference "editor.formatOnSave": false
2. Open a file (launch.json) + adjust the code to be un-formatted + save
3. Open a second file (untitled.ts) + adjust the code to be un-formatted + save
4. reset the preferences "editor.formatOnSave": true + close the preference editor.
5. Put focus on the second file (Untitled.ts)
6. testing with the implemented command : await this.commandServiceFactory().executeCommand('monaco.editor.action.formatDocument');

Menu: "Save All" Result:
==> All files are saved, ONLY the file with the focus (Untitled.ts) is re-formatted
Now start typing anything and the data shows in the focus file (Untitled.ts)

7. testing with the command: await editor.runAction('editor.action.formatDocument');
   Menu: "Save All" Result:
   ==> All files are saved and re-formatted. Focus still on (Untitled.ts) .
   Now start typing anything and the data shows in the other file (launch.json), the other file shows the dirty flag. In the file having the focus (Untitled.ts), it just like it is frozen. Select the "launch.json" , type new code (appear in the focus file) and then select the file "Untitled.ts", now you can type data in it.

[vince-fugnitto]

I'm fine with Save All not being covered in this pr if it is too complicated at the moment.

I see two ways forward:

1. either we complete the ability to Save All for all opened editors including their formatting.
2. we do not cover Save All, and we drop the formatting of the last file in the editor. In my opinion, this behaviour is quite annoying and will definitely lead to unhappy users. (they will attempt to save all the files, and see only a single file actually being formatted without clear knowledge of which file or why).

Also, just curious, with Auto Save set to off, why does the following occur when saving (it only occurs when auto-save is off)?