Skip to content

Commit 6a98c22

Browse files
authored
Hide remove all buttons from trees when there is nothing to remove (#2131)
1 parent 3d6b236 commit 6a98c22

File tree

3 files changed

+154
-15
lines changed

3 files changed

+154
-15
lines changed

extension/package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1213,7 +1213,7 @@
12131213
},
12141214
{
12151215
"command": "dvc.views.experimentsSortByTree.removeAllSorts",
1216-
"when": "view == dvc.views.experimentsSortByTree",
1216+
"when": "view == dvc.views.experimentsSortByTree && dvc.experiments.sorted",
12171217
"group": "navigation@2"
12181218
},
12191219
{
@@ -1223,7 +1223,7 @@
12231223
},
12241224
{
12251225
"command": "dvc.views.experimentsFilterByTree.removeAllFilters",
1226-
"when": "view == dvc.views.experimentsFilterByTree",
1226+
"when": "view == dvc.views.experimentsFilterByTree && dvc.experiments.filtered",
12271227
"group": "navigation@2"
12281228
},
12291229
{

extension/src/context.ts

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Disposable } from './class/dispose'
22
import { CliRunner } from './cli/runner'
3+
import { Experiments } from './experiments'
34
import { WorkspaceExperiments } from './experiments/workspace'
45
import { setContextValue } from './vscode/context'
56

@@ -25,25 +26,59 @@ export class Context extends Disposable {
2526
)
2627
)
2728

29+
this.onDidChangeExperiments()
30+
}
31+
32+
private onDidChangeExperiments() {
2833
this.dispose.track(
2934
this.experiments.onDidChangeExperiments(() => {
30-
this.setIsExperimentRunning()
35+
const repositories: Experiments[] = []
36+
for (const dvcRoot of this.experiments.getDvcRoots()) {
37+
repositories.push(this.experiments.getRepository(dvcRoot))
38+
}
39+
40+
this.setIsExperimentRunning(repositories)
41+
42+
this.setContextFromRepositories(
43+
'dvc.experiments.filtered',
44+
repositories,
45+
(experiments: Experiments) => experiments.getFilters().length > 0
46+
)
47+
48+
this.setContextFromRepositories(
49+
'dvc.experiments.sorted',
50+
repositories,
51+
(experiments: Experiments) => experiments.getSorts().length > 0
52+
)
3153
})
3254
)
3355
}
3456

35-
private setIsExperimentRunning() {
57+
private setIsExperimentRunning(repositories: Experiments[] = []) {
58+
const contextKey = 'dvc.experiment.running'
3659
if (this.cliRunner.isExperimentRunning()) {
37-
setContextValue('dvc.experiment.running', true)
60+
setContextValue(contextKey, true)
3861
return
3962
}
4063

41-
let hasRunning = false
42-
for (const dvcRoot of this.experiments.getDvcRoots()) {
43-
if (this.experiments.getRepository(dvcRoot).hasRunningExperiment()) {
44-
hasRunning = true
64+
this.setContextFromRepositories(
65+
contextKey,
66+
repositories,
67+
(experiments: Experiments) => experiments.hasRunningExperiment()
68+
)
69+
}
70+
71+
private setContextFromRepositories(
72+
contextKey: string,
73+
repositories: Experiments[],
74+
hasRequirement: (experiments: Experiments) => boolean
75+
) {
76+
for (const experiments of repositories) {
77+
if (hasRequirement(experiments)) {
78+
setContextValue(contextKey, true)
79+
return
4580
}
4681
}
47-
setContextValue('dvc.experiment.running', hasRunning)
82+
setContextValue(contextKey, false)
4883
}
4984
}

extension/src/test/suite/context.test.ts

Lines changed: 109 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ import { commands, EventEmitter } from 'vscode'
77
import { CliRunner } from '../../cli/runner'
88
import { WorkspaceExperiments } from '../../experiments/workspace'
99
import { Context } from '../../context'
10+
import { FilterDefinition } from '../../experiments/model/filterBy'
11+
import { SortDefinition } from '../../experiments/model/sortBy'
1012

1113
suite('Context Test Suite', () => {
1214
const disposable = Disposable.fn()
@@ -58,6 +60,18 @@ suite('Context Test Suite', () => {
5860
}
5961
}
6062

63+
const buildMockExperiments = (
64+
filters: FilterDefinition[] = [],
65+
sorts: SortDefinition[] = [],
66+
experimentRunning = false
67+
) => {
68+
return {
69+
getFilters: () => filters,
70+
getSorts: () => sorts,
71+
hasRunningExperiment: () => experimentRunning
72+
}
73+
}
74+
6175
beforeEach(() => {
6276
restore()
6377
})
@@ -105,10 +119,10 @@ suite('Context Test Suite', () => {
105119
mockGetDvcRoots.returns([mockDvcRoot, mockOtherDvcRoot])
106120
mockGetRepository.callsFake(dvcRoot => {
107121
if (dvcRoot === mockDvcRoot) {
108-
return { hasRunningExperiment: () => true }
122+
return buildMockExperiments([], [], true)
109123
}
110124
if (dvcRoot === mockOtherDvcRoot) {
111-
return { hasRunningExperiment: () => false }
125+
return buildMockExperiments()
112126
}
113127
})
114128

@@ -137,9 +151,7 @@ suite('Context Test Suite', () => {
137151

138152
const mockDvcRoot = resolve('first', 'root')
139153
mockGetDvcRoots.returns([mockDvcRoot])
140-
mockGetRepository.callsFake(() => {
141-
return { hasRunningExperiment: () => false }
142-
})
154+
mockGetRepository.callsFake(() => buildMockExperiments())
143155

144156
experimentsChanged.fire()
145157
await experimentsChangedEvent
@@ -150,5 +162,97 @@ suite('Context Test Suite', () => {
150162
false
151163
)
152164
})
165+
166+
it('should set the dvc.experiments.filtered context correctly depending on whether there are filters applied', async () => {
167+
const {
168+
executeCommandSpy,
169+
experimentsChanged,
170+
mockGetDvcRoots,
171+
mockGetRepository,
172+
onDidChangeExperiments
173+
} = buildContext(false)
174+
175+
const firstExperimentsChangedEvent = new Promise(resolve =>
176+
disposable.track(onDidChangeExperiments(() => resolve(undefined)))
177+
)
178+
179+
const mockDvcRoot = resolve('mock', 'root')
180+
mockGetDvcRoots.returns([mockDvcRoot])
181+
mockGetRepository.callsFake(() => buildMockExperiments())
182+
183+
experimentsChanged.fire()
184+
await firstExperimentsChangedEvent
185+
186+
expect(executeCommandSpy).to.be.calledWith(
187+
'setContext',
188+
'dvc.experiments.filtered',
189+
false
190+
)
191+
192+
executeCommandSpy.resetHistory()
193+
mockGetRepository.resetBehavior()
194+
mockGetRepository.callsFake(() =>
195+
buildMockExperiments([{} as FilterDefinition])
196+
)
197+
198+
const secondExperimentsChangedEvent = new Promise(resolve =>
199+
disposable.track(onDidChangeExperiments(() => resolve(undefined)))
200+
)
201+
202+
experimentsChanged.fire()
203+
await secondExperimentsChangedEvent
204+
205+
expect(executeCommandSpy).to.be.calledWith(
206+
'setContext',
207+
'dvc.experiments.filtered',
208+
true
209+
)
210+
})
211+
212+
it('should set the dvc.experiments.sorted context correctly depending on whether there are sorts applied', async () => {
213+
const {
214+
executeCommandSpy,
215+
experimentsChanged,
216+
mockGetDvcRoots,
217+
mockGetRepository,
218+
onDidChangeExperiments
219+
} = buildContext(false)
220+
221+
const firstExperimentsChangedEvent = new Promise(resolve =>
222+
disposable.track(onDidChangeExperiments(() => resolve(undefined)))
223+
)
224+
225+
const mockDvcRoot = resolve('mock', 'root')
226+
mockGetDvcRoots.returns([mockDvcRoot])
227+
mockGetRepository.callsFake(() => buildMockExperiments())
228+
229+
experimentsChanged.fire()
230+
await firstExperimentsChangedEvent
231+
232+
expect(executeCommandSpy).to.be.calledWith(
233+
'setContext',
234+
'dvc.experiments.sorted',
235+
false
236+
)
237+
238+
executeCommandSpy.resetHistory()
239+
mockGetRepository.resetBehavior()
240+
mockGetRepository.callsFake(() =>
241+
buildMockExperiments([], [{} as SortDefinition])
242+
)
243+
244+
const secondExperimentsChangedEvent = new Promise(resolve =>
245+
disposable.track(onDidChangeExperiments(() => resolve(undefined)))
246+
)
247+
248+
experimentsChanged.fire()
249+
await secondExperimentsChangedEvent
250+
251+
expect(executeCommandSpy).to.be.calledWith(
252+
'setContext',
253+
'dvc.experiments.sorted',
254+
true
255+
)
256+
})
153257
})
154258
})

0 commit comments

Comments
 (0)