diff --git a/src/vs/editor/contrib/codeAction/browser/codeAction.ts b/src/vs/editor/contrib/codeAction/browser/codeAction.ts index 9ce2ced9889..9a203b85257 100644 --- a/src/vs/editor/contrib/codeAction/browser/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/browser/codeAction.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import * as nls from 'vs/nls'; import { coalesce, equals, isNonEmptyArray } from 'vs/base/common/arrays'; import { CancellationToken } from 'vs/base/common/cancellation'; import { illegalArgument, isCancellationError, onUnexpectedExternalError } from 'vs/base/common/errors'; @@ -18,7 +19,6 @@ import { ITextModel } from 'vs/editor/common/model'; import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; import { IModelService } from 'vs/editor/common/services/model'; import { TextModelCancellationTokenSource } from 'vs/editor/contrib/editorState/browser/editorState'; -import * as nls from 'vs/nls'; import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands'; import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { INotificationService } from 'vs/platform/notification/common/notification'; @@ -89,6 +89,10 @@ export async function getCodeActions( token: CancellationToken, ): Promise { const filter = trigger.filter || {}; + const notebookFilter: CodeActionFilter = { + ...filter, + excludes: [...(filter.excludes || []), CodeActionKind.Notebook], + }; const codeActionContext: languages.CodeActionContext = { only: filter.include?.value, @@ -96,7 +100,9 @@ export async function getCodeActions( }; const cts = new TextModelCancellationTokenSource(model, token); - const providers = getCodeActionProviders(registry, model, filter); + // if the trigger is auto (autosave, lightbulb, etc), we should exclude notebook codeActions + const excludeNotebookCodeActions = (trigger.type === languages.CodeActionTriggerType.Auto); + const providers = getCodeActionProviders(registry, model, (excludeNotebookCodeActions) ? notebookFilter : filter); const disposables = new DisposableStore(); const promises = providers.map(async provider => { diff --git a/src/vs/editor/contrib/codeAction/common/types.ts b/src/vs/editor/contrib/codeAction/common/types.ts index e1e8d835765..123b4c24c3d 100644 --- a/src/vs/editor/contrib/codeAction/common/types.ts +++ b/src/vs/editor/contrib/codeAction/common/types.ts @@ -20,6 +20,7 @@ export class CodeActionKind { public static readonly RefactorInline = CodeActionKind.Refactor.append('inline'); public static readonly RefactorMove = CodeActionKind.Refactor.append('move'); public static readonly RefactorRewrite = CodeActionKind.Refactor.append('rewrite'); + public static readonly Notebook = new CodeActionKind('notebook'); public static readonly Source = new CodeActionKind('source'); public static readonly SourceOrganizeImports = CodeActionKind.Source.append('organizeImports'); public static readonly SourceFixAll = CodeActionKind.Source.append('fixAll'); diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index e2a4841815e..948c269fa21 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1585,7 +1585,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I InteractiveEditorResponseFeedbackKind: extHostTypes.InteractiveEditorResponseFeedbackKind, StackFrameFocus: extHostTypes.StackFrameFocus, ThreadFocus: extHostTypes.ThreadFocus, - NotebookCodeActionKind: extHostTypes.NotebookCodeActionKind, RelatedInformationType: extHostTypes.RelatedInformationType }; }; diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index f80c0f8d6b3..bead4ad7611 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -7,7 +7,7 @@ import { URI, UriComponents } from 'vs/base/common/uri'; import { equals, mixin } from 'vs/base/common/objects'; import type * as vscode from 'vscode'; import * as typeConvert from 'vs/workbench/api/common/extHostTypeConverters'; -import { Range, Disposable, CompletionList, SnippetString, CodeActionKind, SymbolInformation, DocumentSymbol, SemanticTokensEdits, SemanticTokens, SemanticTokensEdit, Location, InlineCompletionTriggerKind, InternalDataTransferItem, CodeActionTriggerKind, SyntaxTokenType } from 'vs/workbench/api/common/extHostTypes'; +import { Range, Disposable, CompletionList, SnippetString, CodeActionKind, SymbolInformation, DocumentSymbol, SemanticTokensEdits, SemanticTokens, SemanticTokensEdit, Location, InlineCompletionTriggerKind, InternalDataTransferItem, SyntaxTokenType } from 'vs/workbench/api/common/extHostTypes'; import { ISingleEditOperation } from 'vs/editor/common/core/editOperation'; import * as languages from 'vs/editor/common/languages'; import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments'; @@ -377,7 +377,6 @@ class CodeActionAdapter { private readonly _cache = new Cache('CodeAction'); private readonly _disposables = new Map(); - private readonly nbKind = new CodeActionKind('notebook'); constructor( private readonly _documents: ExtHostDocuments, @@ -437,10 +436,6 @@ class CodeActionAdapter { command: this._commands.toInternal(candidate, disposables), }); } else { - if (codeActionContext.triggerKind !== CodeActionTriggerKind.Invoke && candidate.kind && this.nbKind.contains(candidate.kind)) { - continue; - } - if (codeActionContext.only) { if (!candidate.kind) { this._logService.warn(`${this._extension.identifier.value} - Code actions of kind '${codeActionContext.only.value} 'requested but returned code action does not have a 'kind'. Code action will be dropped. Please set 'CodeAction.kind'.`); diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index b1ea9b94055..670dffdcbb4 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -1397,16 +1397,6 @@ export class CodeActionKind { } } -export class NotebookCodeActionKind extends CodeActionKind { - public static override Notebook: CodeActionKind; - - constructor( - public override readonly value: string - ) { - super(value); - } -} - CodeActionKind.Empty = new CodeActionKind(''); CodeActionKind.QuickFix = CodeActionKind.Empty.append('quickfix'); CodeActionKind.Refactor = CodeActionKind.Empty.append('refactor'); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/saveParticipants/saveParticipants.ts b/src/vs/workbench/contrib/notebook/browser/contrib/saveParticipants/saveParticipants.ts index e4d9224bfe3..dc42987bbce 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/saveParticipants/saveParticipants.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/saveParticipants/saveParticipants.ts @@ -38,8 +38,6 @@ import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle import { IStoredFileWorkingCopy, IStoredFileWorkingCopyModel } from 'vs/workbench/services/workingCopy/common/storedFileWorkingCopy'; import { IStoredFileWorkingCopySaveParticipant, IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService'; -const NotebookCodeAction = new CodeActionKind('notebook'); - class FormatOnSaveParticipant implements IStoredFileWorkingCopySaveParticipant { constructor( @IEditorWorkerService private readonly editorWorkerService: IEditorWorkerService, @@ -306,6 +304,7 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa } async participate(workingCopy: IStoredFileWorkingCopy, context: { reason: SaveReason }, progress: IProgress, token: CancellationToken): Promise { + const nbDisposable = new DisposableStore(); const isTrusted = this.workspaceTrustManagementService.isWorkspaceTrusted(); if (!isTrusted) { return; @@ -315,31 +314,46 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa return; } + let saveTrigger = ''; if (context.reason === SaveReason.AUTO) { - return undefined; - } - - const setting = this.configurationService.getValue<{ [kind: string]: boolean } | string[]>(NotebookSetting.codeActionsOnSave); - if (!setting) { + // currently this won't happen, as vs/editor/contrib/codeAction/browser/codeAction.ts L#104 filters out codeactions on autosave. Just future-proofing + // ? notebook CodeActions on autosave seems dangerous (perf-wise) + saveTrigger = 'always'; + } else if (context.reason === SaveReason.EXPLICIT) { + saveTrigger = 'explicit'; + } else { + // SaveReason.FOCUS_CHANGE, WINDOW_CHANGE need to be addressed when autosaves are enabled return undefined; } const notebookModel = workingCopy.model.notebookModel; + const setting = this.configurationService.getValue<{ [kind: string]: string }>(NotebookSetting.codeActionsOnSave); + if (!setting) { + return undefined; + } const settingItems: string[] = Array.isArray(setting) ? setting : Object.keys(setting).filter(x => setting[x]); - if (!settingItems.length) { return undefined; } - const codeActionsOnSave = this.createCodeActionsOnSave(settingItems).filter(x => !NotebookCodeAction.contains(x)); - const notebookCodeActionsOnSave = this.createCodeActionsOnSave(settingItems).filter(x => NotebookCodeAction.contains(x)); + const allCodeActions = this.createCodeActionsOnSave(settingItems); + const excludedActions = allCodeActions + .filter(x => setting[x.value] === 'never'); + const includedActions = allCodeActions + .filter(x => setting[x.value] === saveTrigger); + + const editorCodeActionsOnSave = includedActions.filter(x => !CodeActionKind.Notebook.contains(x)); + const notebookCodeActionsOnSave = includedActions.filter(x => CodeActionKind.Notebook.contains(x)); + if (!editorCodeActionsOnSave.length && !notebookCodeActionsOnSave.length) { + return undefined; + } // prioritize `source.fixAll` code actions if (!Array.isArray(setting)) { - codeActionsOnSave.sort((a, b) => { + editorCodeActionsOnSave.sort((a, b) => { if (CodeActionKind.SourceFixAll.contains(a)) { if (CodeActionKind.SourceFixAll.contains(b)) { return 0; @@ -353,21 +367,6 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa }); } - - - - if (!codeActionsOnSave.length && !notebookCodeActionsOnSave.length) { - return undefined; - } - - const excludedActions = Array.isArray(setting) - ? [] - : Object.keys(setting) - .filter(x => setting[x] === false) - .map(x => new CodeActionKind(x)); - - const nbDisposable = new DisposableStore(); - // run notebook code actions progress.report({ message: localize('notebookSaveParticipants.notebookCodeActions', "Running 'Notebook' code actions") }); try { @@ -387,7 +386,7 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa // run cell level code actions const disposable = new DisposableStore(); - progress.report({ message: localize('notebookSaveParticipants.cellCodeActions', "Running code actions") }); + progress.report({ message: localize('notebookSaveParticipants.cellCodeActions', "Running 'Cell' code actions") }); try { await Promise.all(notebookModel.cells.map(async cell => { const ref = await this.textModelService.createModelReference(cell.uri); @@ -395,7 +394,7 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa const textEditorModel = ref.object.textEditorModel; - await this.applyOnSaveActions(textEditorModel, codeActionsOnSave, excludedActions, progress, token); + await this.applyOnSaveActions(textEditorModel, editorCodeActionsOnSave, excludedActions, progress, token); })); } catch { this.logService.error('Failed to apply code action on save'); @@ -447,7 +446,7 @@ class CodeActionOnSaveParticipant implements IStoredFileWorkingCopySaveParticipa for (const action of actionsToRun.validActions) { const codeActionEdits = action.action.edit?.edits; let breakFlag = false; - if (!action.action.kind?.includes('notebook')) { + if (!action.action.kind?.startsWith('notebook')) { for (const edit of codeActionEdits ?? []) { const workspaceTextEdit = edit as IWorkspaceTextEdit; if (workspaceTextEdit.resource && isEqual(workspaceTextEdit.resource, model.uri)) { diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index b100f521208..c4628df2fd4 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -964,10 +964,14 @@ configurationRegistry.registerConfiguration({ default: false }, [NotebookSetting.codeActionsOnSave]: { - markdownDescription: nls.localize('notebook.codeActionsOnSave', "Experimental. Run a series of CodeActions for a notebook on save. CodeActions must be specified, the file must not be saved after delay, and the editor must not be shutting down. Example: `source.fixAll: true`"), + markdownDescription: nls.localize('notebook.codeActionsOnSave', "Run a series of CodeActions for a notebook on save. CodeActions must be specified, the file must not be saved after delay, and the editor must not be shutting down. Example: `source.fixAll: true`"), type: 'object', additionalProperties: { - type: 'boolean' + type: 'string', + enum: ['explicit', 'never'], + // enum: ['explicit', 'always', 'never'], -- autosave support needs to be built first + // nls.localize('always', 'Always triggers Code Actions on save, including autosave, focus, and window change events.'), + enumDescriptions: [nls.localize('never', 'Never triggers Code Actions on save.'), nls.localize('explicit', 'Triggers Code Actions only when explicitly saved.')], }, default: {} },