Vscode: Consider having a setting to allow tasks and debug to format files when running/debugging

[stefanhaller]

Issue Type: Bug

Steps to Reproduce:

1. Enable editor.formatOnSave, and set task.saveBeforeRun to "always".
2. Hit F5 to start debugging -> dirty files are auto-saved and formatted. Also, code actions are run. (This is expected.)
3. Hit Command-Shift-B to build without running -> dirty files are auto-saved, but _not_ formatted, and no code actions are run. This is not expected.

This is inconsistent; there's no reason why save-before-run and save-before-build should behave differently.

In the code, the reason for this discrepancy is that files are saved with SaveReason.EXPLICIT when debugging, but with SaveReason.AUTO when building. The latter seems wrong to me: the way I understand it, SaveReason.AUTO is for the case that files are auto-saved after a delay, i.e. not in direct response to a user action. In this case it would be confusing when files are modified at arbitrary times, so it makes sense not to run code actions or formatters in that case. However, both running and building are explicit user actions, so it's ok (and desirable) to auto-format code in response.

To sum it up, my proposed fix is to use SaveReason.EXPLICIT when saving files before executing a build task.

VS Code version: Code 1.47.3 (91899dcef7b8110878ea59626991a18c8a6a1b3e, 2020-07-23T13:08:29.692Z)
OS version: Darwin x64 19.6.0

[stefanhaller]

See https://github.com/microsoft/vscode/pull/104298 for a proposed fix.

[alexr00]

@isidorn I deliberately don't have files format when a task build and saves files. I think debug should also not format files in this case.

[isidorn]

I have commented on the PR.
Bottom line: I am fine with changing debug to behave like tasks and am open for a PR for this or I can also do these changes.

[stefanhaller]

Changing debug to no longer auto-format would make it worse for me.

Personally I'm convinced that SaveReason.EXPLICIT is the right choice for both debug and build. You say "It is an explicit gesture, however it is not an explicit gesture to save." That's true, but the user made the choice of turning on task.saveBeforeRun, which means that running (and building) always includes saving. That makes it explicit enough for me.

I would also like to point out that formatting is not the only modification of files that happens when building or debugging. Other modifications include trimming whitespace at line ends, or trimming extra EOLs at the end (if the user has turned these on). And these modifications happen both when building and when debugging.

[alexr00]

Full discussion is here. Debug and tasks should have consistent behavior here, and they should not format by default (#96904).

If there is widespread desire for a setting to configure a format on run task and/or debug, then we would consider adding a setting.

[sterim]

Thanks for looking into this!

I completely agree with @stefanhaller here. And I agree that Debug and Tasks should behave consistently.

In my view 'formatOnSave' makes a general statement that files are always written to disk formatted.
I personally would take this as a guarantee by the IDE that nothing gets written that is not formatted.
A common use case is to use this to ensure company/repository code formatting policies are met.
If some automatic/implicit save operations behave differently from an explicit/manual save, users have no way of telling whether a file on disk is now adhering to formatting rules. This can easily lead to frustrating build breaks on CI systems or rejections by commit hooks that have a checker for policy violations.
So if there are user voices that do not want Debug/Tasks to do an explicit save, I'd vote for an option that can make this explicit, as was suggested in #104298, otherwise formatOnSave basically turns into "maybeFormatOnSave" and loses almost all of its benefits.

[bramankp]

Everyone agrees Debug and Tasks should behave consistently, it seems.

I'm of the opinion that formatOnSave should work for Debug and Tasks by default and if people don't want this to happen there can be an option to disable it. I was quite surprised to have a bunch of files edited and built that had not been properly formatted after several builds.

[edited to add]

If a file auto-saves on Build (and is not formatted) there is no indication that the file still needs to be saved for formatting. Even the "Save All" selection is grayed out in the menu (obviously, because the file has been saved). I just think this is dangerous, especially as others have stated, in corporate environments where formatting rules are explicit.

[edited to also add]

There are two cases where "Save All" could go horribly wrong as a default response to this: 1) I explicitly need to save some files without formatting so I run Save without Formatting on those files before builds, and 2) I have too many tabs open so I close some to reduce clutter and now I've forgotten which files I needed to format explicitly. Can you tell this is really a surprising change to the behavior? I'm wondering if I can downgrade to a previous version and avoid updates somehow so I don't cause problems until this is fixed somehow. :(

[stefanhaller]

Debug and tasks should have consistent behavior here, and they should not format by default (#96904).

If there is widespread desire for a setting to configure a format on run task and/or debug, then we would consider adding a setting.

@alexr00 @isidorn I'd like to politely ask whether there's a chance that you reconsider this decision (or provide more data to justify it).

So far, I have seen exactly _one_ vote for not formatting when running tasks (the one from https://github.com/microsoft/vscode/issues/96904). I have asked over there for clarification what the problem is, but never got a response. So we don't even know why it was requested.

On the other hand, I count 5 votes _for_ formatting on build in this issue alone, with plenty of reasons given. Also, when I talk about this with colleagues at work, all of them agree that files should be formatted on build (as long as saveBeforeRun is on, of course).

I'd be happy to extend #104296 to add an option to turn formatting off for both building and debugging, so that's it's not a regression for @joaomoreno.

What do you think?

[alexr00]

I see the way files get saved as falling into two buckets:

1. An explicit save request from the user
2. An auto save that comes from VS Code. This can have several sources:
   a. files.autoSave setting
   b. Task and debugging
   c. There are probably others too

All saves coming from (1) should be formatted if you have auto formatting on. Easy.

Saves from (2a) _cannot_ be formatted. This is because formatting on auto saves with the setting "files.autoSave": "afterDelay" can change your file while you're working on it. This has been the behavior since the beginning of "files.autoSave": "afterDelay" as far as I know. (2a) is also the largest bucket within (2).

To keep consistency on how we format (2), this means that all saves from (2) should not be auto formatted by default.

I am happy to have a setting that changes auto formatting behavior for (2b).

[stefanhaller]

This is interesting; my classification looks different:

1. Explicit user action
   a. Save file
   b. Run task (when saveBeforeRun is on)
   c. Debug

2. Auto save
   a. after delay
   b. on focus change
   c. on window change

As I said earlier, I consider (1b) and (1c) to be explicit user requests too; this is not something that VS Code does on its own. (Some might even say the same for (2b) and (2c), but it's not quite as obvious for those, so I'm not suggesting to change their behavior.)

Another thought about (1b): if a user sets saveBeforeRun to "prompt", and they run a task and hit the "Save" button in response to the prompt, would you still say that's not an explicit save request? It just seems wrong to me to not auto-format in this case. (And no, I'm not suggesting to make a distinction between "prompt" and "always" here.)

[vscode-triage-bot]

We closed this issue because we don't plan to address it in the foreseeable future. You can find more detailed information about our decision-making process here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!

[stefanhaller]

Yes, I disagree and I have to say that I'm very disappointed at this issue being closed. This _is_ a crucial issue for me, it affects my workflow negatively and makes me less productive.

I created #109857 to address this by adding a new setting to control the behavior, hoping that this is an acceptable compromise.

[stefanhaller]

Since this is the single biggest problem that I have with Visual Studio Code, and there is no satisfactory work-around (see below for more on that), I'd like to make another attempt at convincing you to reopen this issue and solving it in one way or another.

Sorry for the long text, I'm trying to collect as much information about this as I can, and make it as transparent as possible.

Pinging @bpasero and @jrieken just in case they might have any input on this.

A brief recap of what happened

In 2016, the "format on save" feature was implemented, along with the TextDocumentWillSaveEvent event. Back then, it was a conscious decision to format files when they are saved as part of starting a debug session (see this comment).

In May this year, @joaomoreno requested in https://github.com/microsoft/vscode/issues/96904 that files not be formatted when they are saved before running a task, without giving much of a reason for this, other than it being unexpected. The issue was "fixed" in 3257368e97ef, which uses a SaveReason of AUTO when saving files before running a task. I consider this a regression.

That was before I started to use VS Code though, so I didn't realize it was a conscious change. When I started to use VS Code, I soon noticed that running a build task doesn't work the way I expect it to with regards to auto-formatting, so (assuming it was simply a bug) I created an issue for that (this one) and submitted a proposed fix, which happens to basically be a revert of 3257368e97ef.

The PR was rejected, with the reason that instead, debug should be changed to also not auto-format files. This would, of course, make the regression even worse for me.

Meanwhile, a number of people commented on this issue, supporting my position. In contrast, to this day I still only know of one single user who is in favor of not formatting files on running a task, and that user still didn't give any concrete reasons why, even after being explicitly asked about it (see https://github.com/microsoft/vscode/issues/96904#issuecomment-674700719).

Two weeks ago, this issue was closed as "out of scope". In response to that, I created this PR that adds a new config setting to make the behavior optional, hoping that this might be an acceptable compromise. This was rejected as well.

Why is the current behavior a problem?

My usual, day-to-day workflow for writing C++ code is an edit-compile-test cycle where I edit a bunch of files (often several at once), and then hit Command-Shift-B to run my build task, which saves any unsaved files (because task.saveBeforeRun is set to "always"), compiles the program, and runs tests. I rarely hit Command-S manually, because there's no reason to. I only do it when I type some code and I want to see how clang-format formats it (I could use the "Format Document" command instead, but the Command-S shortcut is easier to remember).

So most of the time I rely on "Run task" to save my files, and if it doesn't format them, I am left with unformatted files _almost all the time_. Since we have a pre-push hook that checks that all commits are correctly formatted, I then have to go back and do an interactive rebase to fix the formatting manually, which is tedious and time-consuming.

Workarounds?

• Train myself to type Command-S whenever switching from one file to another. This is what I am currently doing, but it's easy to forget, and I shouldn't have to do it. I don't want to have to adapt to the needs of my tools; my tools should adapt to my needs.
• Train myself to type Command-Option-S (Save All) before building. This is a little better, but in many cases it just doesn't work because I often have untitled files open for keeping notes or snippets; "Save All" asks me where I want to save those, which I don't want to. "Save before run" doesn't do that (which is good), so I really want to use that functionality.
• Write an extension that formats files in response to them being saved with a SaveReason of AUTO. I tried that, but couldn't find a way to format any TextDocument that is not the active editor window. While trying to write such an extension, btw, I saw that listeners of the onWillSaveTextDocument are now called with a TextDocumentSaveReason of AfterDelay when files are saved on running a task, which is clearly wrong. There is no delay in that case.

Wrap-up

None of the workarounds listed above are satisfactory, so some action is needed to solve this problem.

My preferred solution would be to revert 3257368e97ef, because I think it was a mistake. @joaomoreno, could you please give some input about how big of a problem this would be for you, and why.

If that's not possible, the next best solution would be to merge https://github.com/microsoft/vscode/pull/109857. I'm not excited about that, because I think it adds more complexity to the settings window than necessary, but I don't see a third possibility.

Thanks for listening.