Skip to content

Commit 32caf01

Browse files
wolfibDevtools-frontend LUCI CQ
authored andcommitted
[Patch agent] Add saving to workspace
If all modified files have a binding to a file on disk, allow saving the changes to disk. Bug: 406217277 Change-Id: Ib2f0adffc885b467ba885eb00d7b8765a0ba2d9b Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6403240 Commit-Queue: Wolfgang Beyer <[email protected]> Reviewed-by: Ergün Erdoğmuş <[email protected]> Auto-Submit: Wolfgang Beyer <[email protected]>
1 parent 48963ad commit 32caf01

File tree

3 files changed

+78
-34
lines changed

3 files changed

+78
-34
lines changed

front_end/panels/ai_assistance/PatchWidget.test.ts

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,15 +12,14 @@ import {
1212
cleanup,
1313
createPatchWidget,
1414
createPatchWidgetWithDiffView,
15-
createTestFilesystem,
1615
initializePersistenceImplForTests,
1716
MockAidaAbortError,
1817
mockAidaClient,
1918
MockAidaFetchError,
2019
} from '../../testing/AiAssistanceHelpers.js';
2120
import {updateHostConfig} from '../../testing/EnvironmentHelpers.js';
2221
import {describeWithMockConnection} from '../../testing/MockConnection.js';
23-
import {createContentProviderUISourceCode} from '../../testing/UISourceCodeHelpers.js';
22+
import {createContentProviderUISourceCode, createFileSystemUISourceCode} from '../../testing/UISourceCodeHelpers.js';
2423

2524
import * as AiAssistance from './ai_assistance.js';
2625

@@ -37,7 +36,7 @@ describeWithMockConnection('PatchWidget', () => {
3736
cleanup();
3837
});
3938

40-
describe('applyToWorkspace', () => {
39+
describe('applyToPageTree', () => {
4140
beforeEach(() => {
4241
createContentProviderUISourceCode({
4342
url: Platform.DevToolsPath.urlString`file://test/index.html`,
@@ -129,7 +128,7 @@ describeWithMockConnection('PatchWidget', () => {
129128
});
130129
});
131130

132-
it('should show FRE dialog on applyToWorkspace click if the setting is false', async () => {
131+
it('should show FRE dialog on applyToPageTree click if the setting is false', async () => {
133132
Common.Settings.moduleSetting('ai-assistance-patching-fre-completed').set(false);
134133
const {view, widget} = await createPatchWidget();
135134
widget.changeSummary = 'body { background-color: red; }';
@@ -139,7 +138,7 @@ describeWithMockConnection('PatchWidget', () => {
139138
assert.isTrue(showFreDialogStub.called, 'Expected FreDialog to be shown but it\'s not shown');
140139
});
141140

142-
it('should not show FRE dialog on applyToWorkspace click if the setting is true', async () => {
141+
it('should not show FRE dialog on applyToPageTree click if the setting is true', async () => {
143142
Common.Settings.moduleSetting('ai-assistance-patching-fre-completed').set(true);
144143
const {view, widget} = await createPatchWidget();
145144
widget.changeSummary = 'body { background-color: red; }';
@@ -166,7 +165,7 @@ Files:
166165
* /index.html`);
167166
});
168167

169-
it('should show error state when applyToWorkspace fails', async () => {
168+
it('should show error state when applyToPageTree fails', async () => {
170169
const {view, widget} = await createPatchWidget({aidaClient: mockAidaClient([[MockAidaFetchError]])});
171170
widget.changeSummary = 'body { background-color: red; }';
172171

@@ -188,28 +187,47 @@ Files:
188187
});
189188

190189
describe('diff view', () => {
190+
const origContent = 'window.foo = () => "foo";\n';
191191
let fileSystemUISourceCode: Workspace.UISourceCode.UISourceCode;
192192
let commitWorkingCopyStub:
193193
sinon.SinonStub<Parameters<typeof Workspace.UISourceCode.UISourceCode.prototype.commitWorkingCopy>>;
194194
let resetWorkingCopyStub:
195195
sinon.SinonStub<Parameters<typeof Workspace.UISourceCode.UISourceCode.prototype.resetWorkingCopy>>;
196196

197197
beforeEach(() => {
198-
fileSystemUISourceCode = createTestFilesystem('file://test').uiSourceCode;
199-
Common.Settings.Settings.instance().createSetting('ai-assistance-patching-selected-project-id', 'file://test');
200198
updateHostConfig({
201199
devToolsFreestyler: {
202200
enabled: true,
203201
patching: true,
204202
},
205203
});
206204

205+
const url = Platform.DevToolsPath.urlString`https://example.com/script.js`;
206+
createContentProviderUISourceCode({
207+
url,
208+
content: origContent,
209+
mimeType: 'text/javascript',
210+
projectType: Workspace.Workspace.projectTypes.Network,
211+
metadata: new Workspace.UISourceCode.UISourceCodeMetadata(null, origContent.length),
212+
});
213+
207214
commitWorkingCopyStub =
208215
sinon.stub(Workspace.UISourceCode.UISourceCode.prototype, 'commitWorkingCopy').callThrough();
209216
resetWorkingCopyStub =
210217
sinon.stub(Workspace.UISourceCode.UISourceCode.prototype, 'resetWorkingCopy').callThrough();
211218
});
212219

220+
const createBoundFileSystemUISourceCode = () => {
221+
const localUrl = Platform.DevToolsPath.urlString`file:///var/www/script.js`;
222+
({uiSourceCode: fileSystemUISourceCode} = createFileSystemUISourceCode({
223+
url: localUrl,
224+
mimeType: 'text/javascript',
225+
content: origContent,
226+
autoMapping: true,
227+
metadata: new Workspace.UISourceCode.UISourceCodeMetadata(null, origContent.length),
228+
}));
229+
};
230+
213231
it('on apply should call handle function and stash changes', async () => {
214232
const {
215233
view,
@@ -223,22 +241,31 @@ Files:
223241
assert.isTrue(changeManager.stashChanges.calledOnce);
224242
});
225243

226-
it('save all should commit the working copy of the changed UI codes to the disk and render savedToDisk view',
227-
async () => {
228-
const {view, widget} = await createPatchWidgetWithDiffView();
229-
const changeManager = sinon.createStubInstance(AiAssistanceModel.ChangeManager);
230-
widget.changeManager = changeManager;
231-
fileSystemUISourceCode.setWorkingCopy('working copy');
244+
it('"save to workspace" is not available if there is no matching file system mapping', async () => {
245+
const {view, widget} = await createPatchWidgetWithDiffView();
246+
const changeManager = sinon.createStubInstance(AiAssistanceModel.ChangeManager);
247+
widget.changeManager = changeManager;
248+
assert.isUndefined(view.input.onSaveToWorkspace);
249+
});
250+
251+
it('"save to workspace" should commit the working copy of the files to disk and update the view', async () => {
252+
createBoundFileSystemUISourceCode();
253+
fileSystemUISourceCode.setWorkingCopy('working copy');
254+
const {view, widget} = await createPatchWidgetWithDiffView();
255+
const changeManager = sinon.createStubInstance(AiAssistanceModel.ChangeManager);
256+
widget.changeManager = changeManager;
232257

233-
view.input.onSaveAll();
234-
const nextInput = await view.nextInput;
258+
assert.isDefined(view.input.onSaveToWorkspace);
259+
view.input.onSaveToWorkspace();
260+
const nextInput = await view.nextInput;
235261

236-
assert.isTrue(nextInput.savedToDisk);
237-
assert.isTrue(commitWorkingCopyStub.called, 'Expected commitWorkingCopy to be called but it is not called');
238-
assert.isTrue(changeManager.dropStashedChanges.calledOnce);
239-
});
262+
assert.isTrue(nextInput.savedToDisk);
263+
assert.isTrue(commitWorkingCopyStub.called, 'Expected commitWorkingCopy to be called but it is not called');
264+
assert.isTrue(changeManager.dropStashedChanges.calledOnce);
265+
});
240266

241267
it('discard should discard the working copy and render the view without patchSuggestion', async () => {
268+
createBoundFileSystemUISourceCode();
242269
const {view, widget} = await createPatchWidgetWithDiffView();
243270
const changeManager = sinon.createStubInstance(AiAssistanceModel.ChangeManager);
244271
widget.changeManager = changeManager;

front_end/panels/ai_assistance/PatchWidget.ts

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import * as i18n from '../../core/i18n/i18n.js';
1313
import type * as Platform from '../../core/platform/platform.js';
1414
import * as Root from '../../core/root/root.js';
1515
import * as AiAssistanceModel from '../../models/ai_assistance/ai_assistance.js';
16+
import * as Persistence from '../../models/persistence/persistence.js';
1617
import * as WorkspaceDiff from '../../models/workspace_diff/workspace_diff.js';
1718
import * as Buttons from '../../ui/components/buttons/buttons.js';
1819
import * as UI from '../../ui/legacy/legacy.js';
@@ -48,9 +49,9 @@ const UIStringsNotTranslate = {
4849
*/
4950
discard: 'Discard',
5051
/**
51-
*@description Button text to save all the suggested changes to file system
52+
*@description Button text to save all the suggested changes to the file system
5253
*/
53-
saveAll: 'Save all',
54+
saveToWorkspace: 'Save to workspace',
5455
/**
5556
*@description Header text after the user saved the changes to the disk.
5657
*/
@@ -143,7 +144,7 @@ export interface ViewInput {
143144
onApplyToPageTree: () => void;
144145
onCancel: () => void;
145146
onDiscard: () => void;
146-
onSaveAll: () => void;
147+
onSaveToWorkspace?: () => void;
147148
}
148149

149150
export interface ViewOutput {
@@ -167,6 +168,7 @@ export class PatchWidget extends UI.Widget.Widget {
167168
#noLogging: boolean; // Whether the enterprise setting is `ALLOW_WITHOUT_LOGGING` or not.
168169
#patchSuggestionState = PatchSuggestionState.INITIAL;
169170
#workspaceDiff = WorkspaceDiff.WorkspaceDiff.workspaceDiff();
171+
#persistence = Persistence.Persistence.PersistenceImpl.instance();
170172

171173
constructor(element?: HTMLElement, view?: View, opts?: {
172174
aidaClient: Host.AidaClient.AidaClient,
@@ -278,12 +280,14 @@ export class PatchWidget extends UI.Widget.Widget {
278280
.variant=${Buttons.Button.Variant.OUTLINED}>
279281
${lockedString(UIStringsNotTranslate.discard)}
280282
</devtools-button>
281-
<devtools-button
282-
@click=${input.onSaveAll}
283-
.jslogContext=${'patch-widget.save-all'}
284-
.variant=${Buttons.Button.Variant.PRIMARY}>
285-
${lockedString(UIStringsNotTranslate.saveAll)}
286-
</devtools-button>
283+
${input.onSaveToWorkspace ? html`
284+
<devtools-button
285+
@click=${input.onSaveToWorkspace}
286+
.jslogContext=${'patch-widget.save-to-workspace'}
287+
.variant=${Buttons.Button.Variant.PRIMARY}>
288+
${lockedString(UIStringsNotTranslate.saveToWorkspace)}
289+
</devtools-button>
290+
` : nothing}
287291
</div>
288292
</div>
289293
`;
@@ -377,7 +381,7 @@ export class PatchWidget extends UI.Widget.Widget {
377381
this.#applyPatchAbortController?.abort();
378382
},
379383
onDiscard: this.#onDiscard.bind(this),
380-
onSaveAll: this.#onSaveAll.bind(this),
384+
onSaveToWorkspace: this.#canSaveToWorkspace() ? this.#onSaveToWorkspace.bind(this) : undefined,
381385
},
382386
this.#viewOutput, this.contentElement);
383387
}
@@ -487,10 +491,23 @@ ${processedFiles.map(filename => `* ${filename}`).join('\n')}`;
487491
this.requestUpdate();
488492
}
489493

490-
#onSaveAll(): void {
491-
// TODO: What should we do for the inspector stylesheet?
494+
#canSaveToWorkspace(): boolean {
495+
if (this.#patchSuggestionState !== PatchSuggestionState.SUCCESS) {
496+
return false;
497+
}
498+
// TODO(crbug.com/406699819): investigate why the inspector-stylesheet shows up here
499+
const filteredModifiedUISourceCodes =
500+
this.#workspaceDiff.modifiedUISourceCodes().filter(sourceCode => sourceCode.origin() !== 'inspector://');
501+
return filteredModifiedUISourceCodes.length > 0 &&
502+
filteredModifiedUISourceCodes.every(sourceCode => this.#persistence.binding(sourceCode));
503+
}
504+
505+
#onSaveToWorkspace(): void {
492506
this.#workspaceDiff.modifiedUISourceCodes().forEach(modifiedUISourceCode => {
493-
modifiedUISourceCode.commitWorkingCopy();
507+
const binding = this.#persistence.binding(modifiedUISourceCode);
508+
if (binding) {
509+
binding.fileSystem.commitWorkingCopy();
510+
}
494511
});
495512

496513
this.#savedToDisk = true;

front_end/ui/visual_logging/KnownContextValues.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2633,7 +2633,7 @@ export const knownContextValues = new Set([
26332633
'passive',
26342634
'paste',
26352635
'patch-widget.discard',
2636-
'patch-widget.save-all',
2636+
'patch-widget.save-to-workspace',
26372637
'path',
26382638
'pattern',
26392639
'pause',

0 commit comments

Comments
 (0)