Skip to content

vscode.workspace.applyEdit should honour the files.refactoring.autosave config#154079

Merged
jrieken merged 3 commits intomicrosoft:mainfrom
a-stewart:apply-edit-autosave
Sep 8, 2022
Merged

vscode.workspace.applyEdit should honour the files.refactoring.autosave config#154079
jrieken merged 3 commits intomicrosoft:mainfrom
a-stewart:apply-edit-autosave

Conversation

@a-stewart
Copy link
Contributor

@a-stewart a-stewart commented Jul 4, 2022 •
edited
Loading

This PR fixes #112109

This adds a proposed api change, which adds a flag, specifying whether the applyEdit method in the vscode api should honour the files.refactoring.autosave config.

This seems like it would be the appropriate fix for #112109 since we are giving extensions the option to honour the users preference.

I have also added a config option minResourcesToAutosave to the bulk edit options, since by default, it would only auto save if 2 or more files where changed, but this preference would result in some very confusing behaviour for extension authors, so by adding a config option, we can set that to 1 or more files when called by an extension.

One potential issue is that, it seems like autosaving means that files aren't noted as being dirty and therefore don't open by default.

This is more a first pass at a solution than a proposed final result, but it would be good to discuss how you think we should surface this functionality.

@jrieken jrieken self-assigned this Jul 4, 2022
@a-stewart
Copy link
Contributor Author

a-stewart commented Jul 12, 2022

Hi @jrieken - I was wondering if you had an opinion on this? Is this a direction you think it would be good to go in?

@jrieken jrieken added this to the August 2022 milestone Jul 21, 2022
@jrieken
Copy link
Member

jrieken commented Jul 21, 2022

Sorry - I have been swapped recently and I am now heading out to a longer vacation. My plan is to finally pick this up in August. Thanks for being patient.

jrieken
jrieken requested changes Aug 19, 2022
View reviewed changes
Copy link
Member

@jrieken jrieken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. This is looking good already but I left some comments here and there

@jrieken jrieken modified the milestones: August 2022, September 2022 Aug 23, 2022
@jrieken jrieken self-requested a review September 8, 2022 13:22
@jrieken jrieken merged commit ff2d3ca into microsoft:main Sep 8, 2022
@a-stewart
Copy link
Contributor Author

a-stewart commented Sep 8, 2022

Hi,

Sorry I missed your replying to this FR - thanks for applying this change.

A couple of comments:
src/vs/workbench/api/common/extHost.protocol.ts:241
$tryApplyWorkspaceEdit(workspaceEditDto: IWorkspaceEditDto, undoRedoGroupId?: number, respectAutoSaveConfig?: boolean): Promise<boolean>;

Should respectAutoSaveConfig be renamed to isRefactoring? It's an interface so not technical issue, but might be clearer.

Regarding minResourcesToAutosave - the idea behind this was there is a different behaviour if a bulk edit only effects one file. I am not sure if there is a legacy reason for this in the core of VS Code so I didn't want to change anything there, but when auto-safe is requested by an extension, it is slightly confusing if it doesn't apply in some situations.

Perhaps we could remove the > 1 check in src/vs/workbench/contrib/bulkEdit/browser/bulkEditService.ts? But if it is required by the core? Perhaps we could pass the isRefactoring param through and or with that?

@a-stewart a-stewart mentioned this pull request Sep 12, 2022
Open
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jrieken jrieken jrieken approved these changes

@alexr00 alexr00 alexr00 approved these changes

Assignees

@jrieken jrieken

Labels

None yet

Projects

None yet

Milestone

September 2022

Development

Successfully merging this pull request may close these issues.

Add flag to vscode.workspace.applyEdit to save after applying edits

3 participants

@a-stewart @jrieken @alexr00