"Start Debugging" is auto-saving open files even for unchanged files

[spacem]

Since updating today now when I try debugging visual studio codes saves the active file (even though it already is saved) this causes my gulp script to see the change and rebuild/restart node which means node does not show up in the attach to process list. I have "files.autoSave": "off" in my config.

• VSCode Version: 1.10.1
• OS Version: Windows 10

Steps to Reproduce:

1. Open a js file
2. Click the green play button in the debug view

[isidorn]

@spacem yes this is how start debugging is behaving by design. We could introduce an configuraiton to not autosave on start.
@bpasero shouldn't the save operation be a no-op when there are no changes?

[bpasero]

@isidorn I suggest you add a breakpoint here to find out why it saves, the reasons are explained in that location: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/services/textfile/common/textFileEditorModel.ts#L600

[bpasero]

Another case where a save can cause something to happen is when a save participant is installed that wants to change the file (even if it is not dirty).

[adelphes]

This issue has broken my incremental build feature.

Saving clean files when launching a debug session is not a good idea. Like many designs, my debugger tests whether a rebuild is necessary by comparing the file times of the source files with the file times of the output. If the source files are newer, a rebuild is necessary.

Performing a save on every debug session causes the source files to have a later timestamp, forcing a rebuild every time.

Can you revert this back to the original behaviour.

[alemhnan]

Also not sure if it's related, but now nodemon restart the server when I start to debug. The net result is that I get a timeout error everytime now (related bug #21783)

[isidorn]

@bpasero what changed is that now I am explictly calling the workbench.action.files.save due to #19415
In textFileEditorModel line 600 all those fields are true because the save action wants to trigger all external file watchers. And that is bascially the difference between the workbench.action.files.save and calling saveAll on the fileService as we had done before.

@adelphes @alemhnan @spacem yes, we will revert back to the original behavior and you can pick up the fix in tomorrow's vscode insiders.
For now I am not sure if this should be treated as a candidate for the recovery build. @bpasero opinion?

[bpasero]

@isidorn what about we introduce another save action which is "Save if Dirty" and all it does is to be different around this line where it forces a write even if the file is not dirty? People that do not want our current behaviour for saving could even remap the Ctrl+S to that command.

[isidorn]

@bpasero sounds good to me. I can look into it later tonight.

[isidorn]

@bpasero I introduced a new Save if Dirty action, currently I registered as a regular action so it appears in the command palette which I am not sure is the best.

Adding candidate label to discuss if this should be a candidate in tomorrow's standup.

@adelphes @alemhnan @spacem tomorrow's insiders will contain the fix for the issue. Can you please try it out. Thanks

[spacem]

Sorry I do not know what you mean by "tomorrow's insiders". I did a "check for updates" and it said none were found.

One thing which maybe is a different issue is I tried just closing all my open windows and starting debug. This didn't save any files however it gives the error "Attach to process: '${command.PickProcess}' doesn't look like a process id.".

[bpasero]

@isidorn good point, alternatively we could introduce a command with "_" to exclude it from the F1 list until someone asks for it.

[isidorn]

@spacem VS Code also has an insiders version which updates every day. You can get it from here http://code.visualstudio.com/docs/?dv=win&build=insiders

@bpasero I will remove the '...' from the label of the command - 3448016

[adelphes]

Tested with today's insider build...
2017-03-03T07:08:55.738Z
84e56fc

...and it works. It no longer saves clean files when launching a debug session.

A slightly confusing behaviour linked to this is that if you have multiple dirty files open, only the currently active editor is saved when debugging is started. Is this expected?

[isidorn]

@adelphes good point. Looking into that I figured out that a neater solution is to simply call 'workbench.action.files.saveAll' action that does everything correctly. Saves all and does not force a write. I have pushed this and will be available in tomorrow's insiders. It will also be availble in VSCode stable some time next week.

fyi @bpasero

[roblourens]

Now when I have an untitled file open, it brings up a save dialog every time I start debugging. I frequently use untitled files as scratch pads, so this is annoying. How about saveAllIfDirty?

[bpasero]

@isidorn you can use workbench.action.files.saveFiles to only save files, but I thought this feature was about being able to debug even an untitled file?

[isidorn]

@roblourens this feature was about being able to debug untitiled files #19415. Let's wait if somebody else complains other than Rob and we can look into improving this. ATM treating this as designed behavior

[roblourens]

Ok, I'll get someone else to complain about it :)

It's really annoying...

[chrmarti]

Verified neither the current nor another file get an updated modification timestamp unless they are dirty when starting to debug.

[kieferrm]

For users who are no relying on hot-exit - as I do - this is the old behavior right after F5:

This is the new behavior. Every time I now asked to save, although I don't intend to save any of the untitled files. This is super annoying.

[roblourens]

Like Ben suggested, calling workbench.action.files.saveFiles at https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/browser/debugActions.ts#L130 will save all open dirty files, and won't save untitled files.

We could also remove that call to not save at all when starting to debug, and assume that people will use autosave if they want it to save there.

[chrmarti]

Having talked with @Tyriar about his use of the dirty state I'd vote for not saving at all. Auto-save can be used when not interested in managing the dirty state manually. (Disclaimer: I'm using auto-save.)

[bpasero]

@isidorn to preserve most of the behaviour we want and NOT trigger the "save even if not dirty", I suggest you bring back the action we had in the first place which is to subclass the save action and not force it to save unless the file is dirty. To make this impact minimal, I suggest to register the action in the same way we do it for SaveFilesAction in https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/files/browser/fileActions.contribution.ts#L215 so that it does not show up in the F1 list.

[isidorn]

Now we are only saving the current active file if not dirty before debugging, which is a slight change as to what we were doing before but I think it makes a lot of sense overall. We will trigger new builds after standup since there are other potential candidates to discuss.
@bpasero please code review and verify

[isidorn]

After discussions we have decided to go back to the old behavior we were doing in the previous releases. We are saving all untitiled non dirty files before debug starts.
This is without a behavioral change and has the least amount of risk associated with it. The only downside is that we had to reopen #19415

@bpasero please code review and verify