Skip to content

Commit

Permalink
add workspace focus when params file is open
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed May 20, 2022
1 parent f97ca0a commit 92872b7
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 7 deletions.
11 changes: 10 additions & 1 deletion extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export const ExperimentsScale = {
} as const

export class Experiments extends BaseRepository<TableData> {
public readonly onDidChangeIsParamsFileFocused: Event<string | undefined>
public readonly onDidChangeExperiments: Event<ExperimentsOutput | void>
public readonly onDidChangeColumns: Event<void>
public readonly onDidChangeCheckpoints: Event<void>
Expand All @@ -57,6 +58,10 @@ export class Experiments extends BaseRepository<TableData> {
private readonly columns: ColumnsModel
private readonly checkpoints: CheckpointsModel

private readonly paramsFileFocused = this.dispose.track(
new EventEmitter<string | undefined>()
)

private readonly experimentsChanged = this.dispose.track(
new EventEmitter<ExperimentsOutput | void>()
)
Expand All @@ -82,6 +87,7 @@ export class Experiments extends BaseRepository<TableData> {

this.internalCommands = internalCommands

this.onDidChangeIsParamsFileFocused = this.paramsFileFocused.event
this.onDidChangeExperiments = this.experimentsChanged.event
this.onDidChangeColumns = this.columnsChanged.event
this.onDidChangeCheckpoints = this.checkpointsChanged.event
Expand Down Expand Up @@ -542,8 +548,11 @@ export class Experiments extends BaseRepository<TableData> {
}

private setActiveEditorContext() {
const setActiveEditorContext = (active: boolean) =>
const setActiveEditorContext = (active: boolean) => {
setContextValue('dvc.params.fileActive', active)
const activeDvcRoot = active ? this.dvcRoot : undefined
this.paramsFileFocused.fire(activeDvcRoot)
}

this.dispose.track(
this.onDidChangeColumns(() => {
Expand Down
17 changes: 17 additions & 0 deletions extension/src/experiments/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<

private readonly checkpointsChanged: EventEmitter<void>

private focusedParamsDvcRoot: string | undefined

constructor(
internalCommands: InternalCommands,
updatesPaused: EventEmitter<boolean>,
Expand Down Expand Up @@ -232,6 +234,13 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
dvcRoot => (this.focusedWebviewDvcRoot = dvcRoot)
)
)

experiments.dispose.track(
experiments.onDidChangeIsParamsFileFocused(
dvcRoot => (this.focusedParamsDvcRoot = dvcRoot)
)
)

experiments.dispose.track(
experiments.onDidChangeExperiments(() => {
this.experimentsChanged.fire()
Expand All @@ -253,6 +262,14 @@ export class WorkspaceExperiments extends BaseWorkspaceWebviews<
return experiments
}

public getFocusedOrOnlyOrPickProject() {
return (
this.focusedWebviewDvcRoot ||
this.focusedParamsDvcRoot ||
this.getOnlyOrPickProject()
)
}

private async pickExpThenRun(
commandId: CommandId,
pickFunc: (
Expand Down
4 changes: 4 additions & 0 deletions extension/src/plots/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,8 @@ export class WorkspacePlots extends BaseWorkspaceWebviews<Plots, PlotsData> {
}
return this.getRepository(dvcRoot).selectPlots()
}

public getFocusedOrOnlyOrPickProject() {
return this.focusedWebviewDvcRoot || this.getOnlyOrPickProject()
}
}
2 changes: 1 addition & 1 deletion extension/src/test/suite/experiments/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export const buildMultiRepoExperiments = (disposer: Disposer) => {
resourceLocator
)
experiments.setState(expShowFixture)
return { experiments, messageSpy, workspaceExperiments }
return { experiments, internalCommands, messageSpy, workspaceExperiments }
}

export const buildSingleRepoExperiments = (disposer: Disposer) => {
Expand Down
52 changes: 50 additions & 2 deletions extension/src/test/suite/experiments/workspace.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { afterEach, beforeEach, describe, it, suite } from 'mocha'
import { expect } from 'chai'
import { stub, restore, SinonStub, match } from 'sinon'
import { window, commands, QuickPickItem } from 'vscode'
import { window, commands, QuickPickItem, Uri } from 'vscode'
import {
buildExperiments,
buildMultiRepoExperiments,
Expand All @@ -27,6 +27,8 @@ import {
} from '../../../vscode/quickPick'
import { WEBVIEW_TEST_TIMEOUT } from '../timeouts'
import { Title } from '../../../vscode/title'
import { join } from '../../util/path'
import { AvailableCommands } from '../../../commands/internal'

suite('Workspace Experiments Test Suite', () => {
const disposable = Disposable.fn()
Expand All @@ -52,7 +54,7 @@ suite('Workspace Experiments Test Suite', () => {
)
})

describe('showExperimentsTable', () => {
describe('project focus', () => {
it('should prompt to pick a project even if a webview is focused', async () => {
const mockQuickPickOne = stub(QuickPick, 'quickPickOne').resolves(
dvcDemoPath
Expand Down Expand Up @@ -91,6 +93,52 @@ suite('Workspace Experiments Test Suite', () => {

expect(mockQuickPickOne).to.not.be.called
})

it('should not prompt to pick a project if a params file is focused', async () => {
const mockQuickPickOne = stub(QuickPick, 'quickPickOne').resolves(
dvcDemoPath
)

const { workspaceExperiments, experiments, internalCommands } =
buildMultiRepoExperiments(disposable)

await workspaceExperiments.isReady()

const focusedWebview = onDidChangeIsWebviewFocused(experiments)

await workspaceExperiments.showWebview()

expect(await focusedWebview).to.equal(dvcDemoPath)

const focusedParamsFile = new Promise(resolve => {
const listener: Disposable = experiments.onDidChangeIsParamsFileFocused(
(event: string | undefined) => {
listener.dispose()
return resolve(event)
}
)
})

const paramsFile = Uri.file(join(dvcDemoPath, 'params.yaml'))
await window.showTextDocument(paramsFile)

expect(await focusedParamsFile).to.equal(dvcDemoPath)

mockQuickPickOne.resetHistory()

const mockExecuteCommand = stub(
internalCommands,
'executeCommand'
).resolves(undefined)

await workspaceExperiments.getCwdThenRun(AvailableCommands.EXPERIMENT_RUN)

expect(mockQuickPickOne).not.to.be.calledOnce
expect(mockExecuteCommand).to.be.calledWith(
AvailableCommands.EXPERIMENT_RUN,
dvcDemoPath
)
})
}).timeout(WEBVIEW_TEST_TIMEOUT)

describe('dvc.modifyExperimentParamsAndQueue', () => {
Expand Down
4 changes: 1 addition & 3 deletions extension/src/webview/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,5 @@ export abstract class BaseWorkspaceWebviews<
return overrideRoot || (await this.getFocusedOrOnlyOrPickProject())
}

protected getFocusedOrOnlyOrPickProject() {
return this.focusedWebviewDvcRoot || this.getOnlyOrPickProject()
}
abstract getFocusedOrOnlyOrPickProject(): string | Promise<string | undefined>
}

0 comments on commit 92872b7

Please sign in to comment.