editor.formatOnSaveMode option is not respected

[BerserkWolfS]

Hello,

firstly, thanks for your great extension and the efforts you put into it, I found the new formatter efficient and easy to use and configure. I have a request though regarding the option editor.formatOnSave. In my team, we configure it to modifications only to ease our code reviews, but the extensions seems to modify the whole file. Is it a side effect, have I missed a setting or is it planned for a future release ?
This is my vscode configuration:

Version: 1.87.2
Commit: 863d2581ecda6849923a2118d93a088b0745d9d6
Date: 2024-03-08T15:20:17.278Z
Electron: 27.3.2
ElectronBuildId: 26836302
Chromium: 118.0.5993.159
Node.js: 18.17.1
V8: 11.8.172.18-electron.0
OS: Windows_NT x64 10.0.22621

XML extension: 0.26.1

Thanks

[angelozerr]

We are glad that vscode xml please you. Thanks for your feedback!

To be honnest with you I have never used this format on save options.

I dont think we have implemented something for this option.

When you activate this options it should format the whole file, right?

Im sorry I dont understand your problem?

[BerserkWolfS]

Hello Angelo,

Thanks for your quick reply. In fact, the editor.formatOnSaveMode option (I corrected the name in the title since I forgot the "Mode" part) is an enum that you can set to modifications, which means only the lines you modified in a file will be formatted. I checked the tooltip, it says that is requires source control, so maybe your extension may have to use this information somehow ? Or maybe it is provided by vscode api, I don't know.

For instance, let's says I have the following content in a xml file:

<example>
  <par> 'Content' </par>
  <par> 'Other' </par>
</example>

Then I make a small modification to the line containing the word 'Content'. Imagine I turned on the enforce quote style option to preferred in the xml extension, and my preference is to use double quotes. Then, only the modified line should be formatted, like this:

<example>
  <par> "Content" modified </par>
  <par> 'Other' </par>
</example>

I know it is not ideal because it means that the file will end up containing mixed style. But it is the responsibility of our team to either keep it like this to ease the code review later and ensure that the rest of the file is kept intact or to fully format it using the format file command on purpose.

Do you think you can either add an option or take into account the built-in editor.formatOnSaveMode ?

[fbricon]

It works for me (enforce quote style refers to quotes in attributes)

Apr-25-2024.15-41-46.mp4

[BerserkWolfS]

ok, so I had to complexify my example a little to reproduce the issue ^^.

Here is the new sample:

<?xml version="1.0" ?>
<example>
    <par attr='myattr2'> Other </par>
    <par attr='myattr3'>
        <subpar attr='mysubattr1'/>
        <subpar attr='mysubattr2'/>
    </par>
    <par attr='myattr1'> Content </par>
</example>

2024-04-25_16-12-55.mp4

As you can see, the subpar are also formatted. Maybe it is a designed choice ?

[fbricon]

No that looks like a bug

[fbricon]

@angelozerr I'm adding the lemminx trace for posterity:

[Trace - 4:32:24 PM] Sending request 'textDocument/rangeFormatting - (66)'.
Params: {
    "textDocument": {
        "uri": "file:///Users/fbricon/Dev/souk/spring-petclinic/example.xml"
    },
    "range": {
        "start": {
            "line": 7,
            "character": 0
        },
        "end": {
            "line": 7,
            "character": 47
        }
    },
    "options": {
        "tabSize": 4,
        "insertSpaces": true,
        "trimFinalNewlines": true
    }
}


[Trace - 4:32:24 PM] Received response 'textDocument/rangeFormatting - (66)' in 1ms.
Result: [
    {
        "range": {
            "start": {
                "line": 4,
                "character": 21
            },
            "end": {
                "line": 4,
                "character": 22
            }
        },
        "newText": "\""
    },
    {
        "range": {
            "start": {
                "line": 4,
                "character": 32
            },
            "end": {
                "line": 4,
                "character": 33
            }
        },
        "newText": "\""
    },
    {
        "range": {
            "start": {
                "line": 4,
                "character": 33
            },
            "end": {
                "line": 4,
                "character": 33
            }
        },
        "newText": " "
    },
    {
        "range": {
            "start": {
                "line": 5,
                "character": 21
            },
            "end": {
                "line": 5,
                "character": 22
            }
        },
        "newText": "\""
    },
    {
        "range": {
            "start": {
                "line": 5,
                "character": 32
            },
            "end": {
                "line": 5,
                "character": 33
            }
        },
        "newText": "\""
    },
    {
        "range": {
            "start": {
                "line": 5,
                "character": 33
            },
            "end": {
                "line": 5,
                "character": 33
            }
        },
        "newText": " "
    },
    {
        "range": {
            "start": {
                "line": 7,
                "character": 14
            },
            "end": {
                "line": 7,
                "character": 15
            }
        },
        "newText": "\""
    },
    {
        "range": {
            "start": {
                "line": 7,
                "character": 22
            },
            "end": {
                "line": 7,
                "character": 23
            }
        },
        "newText": "\""
    }
]


[Trace - 4:32:24 PM] Sending notification 'textDocument/didChange'.
Params: {
    "textDocument": {
        "uri": "file:///Users/fbricon/Dev/souk/spring-petclinic/example.xml",
        "version": 13
    },
    "contentChanges": [
        {
            "range": {
                "start": {
                    "line": 7,
                    "character": 22
                },
                "end": {
                    "line": 7,
                    "character": 23
                }
            },
            "rangeLength": 1,
            "text": "\""
        },
        {
            "range": {
                "start": {
                    "line": 7,
                    "character": 14
                },
                "end": {
                    "line": 7,
                    "character": 15
                }
            },
            "rangeLength": 1,
            "text": "\""
        },
        {
            "range": {
                "start": {
                    "line": 5,
                    "character": 32
                },
                "end": {
                    "line": 5,
                    "character": 33
                }
            },
            "rangeLength": 1,
            "text": "\" "
        },
        {
            "range": {
                "start": {
                    "line": 5,
                    "character": 21
                },
                "end": {
                    "line": 5,
                    "character": 22
                }
            },
            "rangeLength": 1,
            "text": "\""
        },
        {
            "range": {
                "start": {
                    "line": 4,
                    "character": 32
                },
                "end": {
                    "line": 4,
                    "character": 33
                }
            },
            "rangeLength": 1,
            "text": "\" "
        },
        {
            "range": {
                "start": {
                    "line": 4,
                    "character": 21
                },
                "end": {
                    "line": 4,
                    "character": 22
                }
            },
            "rangeLength": 1,
            "text": "\""
        }
    ]
}


[Trace - 4:32:24 PM] Sending notification 'textDocument/didSave'.
Params: {
    "textDocument": {
        "uri": "file:///Users/fbricon/Dev/souk/spring-petclinic/example.xml"
    }
}

[angelozerr]

It seems that there is a but with range formatting. If you select the whole line and you format range you should have the same problem.