fix #3338 Format Document on save

[lmcbout]

Fix #3338
Depends on issue #3617
Note: If #3617 not available, then the java file will need to be saved twice when the user wants to format the file and cleaning up the lines from empty spaces.

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

[elaihau]

this.getEditsTrimmingTrailingWhitespaces() also returns "edit" that should be pushed in to edits. I believe it is missing in the change.

[lmcbout]

Done, adjusted the return value

[vince-fugnitto]

@lmcbout why does formatOnSave only work when auto-save is off?
In vscode, if I manually Save the formatter also kicks in.

[elaihau]

Tested. Works.

[lmcbout]

@vince-fugnitto If we allow to formatOnSave when the auto-save = "on", depending of the file type it becomes annoying to see the empty space or empty line goes away before you type new chars into the file because the file has been saved and re-formatted

[vince-fugnitto]

@vince-fugnitto If we allow to formatOnSave when the auto-save = "on", depending of the file type it becomes annoying to see the empty space or empty line goes away before you type new chars into the file because the file has been saved and re-formatted

@lmcbout formatOnSave should only be triggered when we explicitly call Save either from the menu, or the keybinding. The auto-save feature does not call the formatter. That's the way vscode does it.

[lmcbout]

@akosyakov Do you have any comments before we merge it since it has already been approved.
Thanks

[elaihau]

this should be broken into multiple lines :)

[lmcbout]

Agree, Done

[vince-fugnitto]

@lmcbout I don't think we should limit ourselves to autoSave on or off.
We should call the formatter when the user explicitly calls Save.
The auto-save should not call the formatter and thus the problem you mentioned won't exist.

[akosyakov]

Why is it related to editorconfig extension? It should work even without this extension, only with editor + monaco extensions.

[elaihau]

@akosyakov could you please suggest which package this logic belongs to, if not put into editorconfig?
editorconfig already does things like "trim tailing whitespace" and "adding a new line", and it also has a .editorconfig that controls the things to be done depending on the file extension.

why do we not want to put all the related things in one place ?

[akosyakov]

could you please suggest which package this logic belongs to, if not put into editorconfig?

Ideally, it should go to the editor extension, but since it is relays on monaco apis i would implement it in the monaco extension.

editorconfig already does things like "trim tailing whitespace" and "adding a new line", and it also has a .editorconfig that controls the things to be done depending on the file extension.

editorconfig extension is responsible for additional formatting based on .editorconfig file, not for general handling of formatting

[paul-marechal]

As Anton said, this change shouldn't live in editorconfig.

Also, after further investigation, it would seem that onSave contributions such as edits are not correctly handled in Theia as of now. Mostly concurrency issues.