Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Experiment table open to the side #1796

Merged
merged 9 commits into from
Jun 2, 2022
5 changes: 3 additions & 2 deletions extension/src/experiments/columns/paths.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ export const splitColumnPath = (path: string) => {
if (!fileSegment) {
return [baseSegment]
}
const cleanFileSegment = fileSegment.replace(/_\d+$/g, '')
if (!paramPath) {
return [baseSegment, fileSegment]
return [baseSegment, cleanFileSegment]
}
return [
baseSegment,
fileSegment,
cleanFileSegment,
...paramPath.split(METRIC_PARAM_SEPARATOR).map(decodeColumn)
]
}
18 changes: 17 additions & 1 deletion extension/src/experiments/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Event, EventEmitter, Memento, window } from 'vscode'
import { join } from 'path'
import { Event, EventEmitter, Memento, Uri, ViewColumn, window } from 'vscode'
import { ExperimentsModel } from './model'
import { pickExperiments } from './model/quickPicks'
import { pickAndModifyParams } from './model/queue/quickPick'
Expand All @@ -15,6 +16,7 @@ import { ExperimentsData } from './data'
import { askToDisableAutoApplyFilters } from './toast'
import { Experiment, ColumnType, TableData } from './webview/contract'
import { SortDefinition } from './model/sortBy'
import { splitColumnPath } from './columns/paths'
import { ResourceLocator } from '../resourceLocator'
import {
AvailableCommands,
Expand Down Expand Up @@ -380,6 +382,18 @@ export class Experiments extends BaseRepository<TableData> {
)
}

private async openParamsFileToTheSide(path: string) {
const [, fileSegment] = splitColumnPath(path)
await window.showTextDocument(Uri.file(join(this.dvcRoot, fileSegment)), {
Copy link
Member

Choose a reason for hiding this comment

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

[I] Potentially we could rename and re-use RegisteredCommands.TRACKED_EXPLORER_OPEN_TO_THE_SIDE here. If not this should have its own analytics event.

Copy link
Member

Choose a reason for hiding this comment

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

Probably better to just give this its own event.

viewColumn: ViewColumn.Beside
})
sendTelemetryEvent(
EventName.VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE,
{ path },
undefined
)
}

private setupInitialData() {
const waitForInitialData = this.dispose.track(
this.onDidChangeExperiments(() => {
Expand Down Expand Up @@ -440,6 +454,8 @@ export class Experiments extends BaseRepository<TableData> {
return this.setExperimentStatus(message.payload)
case MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN:
return this.hideTableColumn(message.payload)
case MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE:
return this.openParamsFileToTheSide(message.payload)
case MessageFromWebviewType.SORT_COLUMN:
return this.addColumnSort(message.payload)
case MessageFromWebviewType.REMOVE_COLUMN_SORT:
Expand Down
5 changes: 5 additions & 0 deletions extension/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ export const EventName = Object.assign(
VIEWS_EXPERIMENTS_TABLE_FOCUS_CHANGED:
'views.experimentsTable.focusChanged',
VIEWS_EXPERIMENTS_TABLE_HIDE_COLUMN: 'views.experimentsTable.columnHidden',
VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE:
'views.experimentsTable.paramsFileOpened',
VIEWS_EXPERIMENTS_TABLE_REMOVE_COLUMN_SORT:
'views.experimentsTable.columnSortRemoved',
VIEWS_EXPERIMENTS_TABLE_RESIZE_COLUMN:
Expand Down Expand Up @@ -196,6 +198,9 @@ export interface IEventNamePropertyMapping {
path: string
}
[EventName.VIEWS_EXPERIMENTS_TABLE_SELECT_COLUMNS]: undefined
[EventName.VIEWS_EXPERIMENTS_TABLE_OPEN_PARAMS_FILE]: {
path: string
}

[EventName.VIEWS_PLOTS_CLOSED]: undefined
[EventName.VIEWS_PLOTS_CREATED]: undefined
Expand Down
47 changes: 46 additions & 1 deletion extension/src/test/suite/experiments/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
commands,
workspace,
Uri,
QuickPickItem
QuickPickItem,
ViewColumn
} from 'vscode'
import { buildExperiments } from './util'
import { Disposable } from '../../../extension'
Expand Down Expand Up @@ -390,6 +391,50 @@ suite('Experiments Test Suite', () => {
)
})

it('should be able to handle a message to open the source params file from a column path', async () => {
const { experiments } = setupExperimentsAndMockCommands()

const mockShowTextDocument = stub(window, 'showTextDocument')
const webview = await experiments.showWebview()
const mockMessageReceived = getMessageReceivedEmitter(webview)
const mockColumnId = 'params:params.yaml_5'

mockMessageReceived.fire({
payload: mockColumnId,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
})

expect(mockShowTextDocument).to.be.calledOnce
expect(mockShowTextDocument).to.be.calledWithExactly(
Uri.file(join(dvcDemoPath, 'params.yaml')),
{
viewColumn: ViewColumn.Beside
}
)
})

it('should be able to handle a message to open different params files than the default one', async () => {
const { experiments } = setupExperimentsAndMockCommands()

const mockShowTextDocument = stub(window, 'showTextDocument')
const webview = await experiments.showWebview()
const mockMessageReceived = getMessageReceivedEmitter(webview)
const mockColumnId = 'params:params_alt.json_5:nested1.nested2'

mockMessageReceived.fire({
payload: mockColumnId,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
})

expect(mockShowTextDocument).to.be.calledOnce
expect(mockShowTextDocument).to.be.calledWithExactly(
Uri.file(join(dvcDemoPath, 'params_alt.json')),
{
viewColumn: ViewColumn.Beside
}
)
})

it('should be able to handle a message to apply an experiment to workspace', async () => {
const { experiments, mockExecuteCommand } =
setupExperimentsAndMockCommands()
Expand Down
5 changes: 5 additions & 0 deletions extension/src/webview/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export enum MessageFromWebviewType {
INITIALIZED = 'initialized',
APPLY_EXPERIMENT_TO_WORKSPACE = 'apply-experiment-to-workspace',
CREATE_BRANCH_FROM_EXPERIMENT = 'create-branch-from-experiment',
OPEN_PARAMS_FILE_TO_THE_SIDE = 'open-params-file-to-the-side',
REMOVE_COLUMN_SORT = 'remove-column-sort',
REMOVE_EXPERIMENT = 'remove-experiment',
RENAME_SECTION = 'rename-section',
Expand Down Expand Up @@ -68,6 +69,10 @@ export type MessageFromWebview =
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
payload: string
}
| {
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
payload: string
}
| {
type: MessageFromWebviewType.APPLY_EXPERIMENT_TO_WORKSPACE
payload: string
Expand Down
14 changes: 6 additions & 8 deletions webview/src/experiments/components/table/Row.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { sendMessage } from '../../../shared/vscode'
import { ContextMenu } from '../../../shared/components/contextMenu/ContextMenu'
import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu'
import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption'
import { pushIf } from '../../../util/array'

const getExperimentTypeClass = ({ running, queued, selected }: Experiment) => {
if (running) {
Expand Down Expand Up @@ -52,10 +53,7 @@ export const RowContextMenu: React.FC<RowProp> = ({
const contextMenuOptions = React.useMemo(() => {
const menuOptions: MessagesMenuOptionProps[] = []

const pushIf = (condition: boolean, options: MessagesMenuOptionProps[]) =>
condition && menuOptions.push(...options)

pushIf(!queued && !isWorkspace && depth > 0, [
pushIf(menuOptions, !queued && !isWorkspace && depth > 0, [
experimentMenuOption(
id,
'Apply to Workspace',
Expand All @@ -68,7 +66,7 @@ export const RowContextMenu: React.FC<RowProp> = ({
)
])

pushIf(depth === 1, [
pushIf(menuOptions, depth === 1, [
experimentMenuOption(
id,
'Remove',
Expand All @@ -78,23 +76,23 @@ export const RowContextMenu: React.FC<RowProp> = ({

const isNotCheckpoint = depth <= 1 || isWorkspace

pushIf(isNotCheckpoint, [
pushIf(menuOptions, isNotCheckpoint, [
experimentMenuOption(
id,
projectHasCheckpoints ? 'Modify and Resume' : 'Modify and Run',
MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_AND_RUN
)
])

pushIf(isNotCheckpoint && projectHasCheckpoints, [
pushIf(menuOptions, isNotCheckpoint && projectHasCheckpoints, [
experimentMenuOption(
id,
'Modify, Reset and Run',
MessageFromWebviewType.VARY_EXPERIMENT_PARAMS_RESET_AND_RUN
)
])

pushIf(isNotCheckpoint, [
pushIf(menuOptions, isNotCheckpoint, [
experimentMenuOption(
id,
'Modify and Queue',
Expand Down
59 changes: 39 additions & 20 deletions webview/src/experiments/components/table/TableHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
OnDrop
} from '../../../shared/components/dragDrop/DragDropWorkbench'
import { MessagesMenu } from '../../../shared/components/messagesMenu/MessagesMenu'
import { MessagesMenuOptionProps } from '../../../shared/components/messagesMenu/MessagesMenuOption'

export const ColumnDragHandle: React.FC<{
disabled: boolean
Expand Down Expand Up @@ -197,6 +198,31 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
})
}

const contextMenuOptions: MessagesMenuOptionProps[] = React.useMemo(() => {
const menuOptions: MessagesMenuOptionProps[] = [
{
hidden: !!column.headers,
id: 'hide-column',
label: 'Hide Column',
message: {
payload: column.id,
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
}
},
{
hidden: column.group !== ColumnType.PARAMS,
id: 'open-to-the-side',
label: 'Open to the Side',
message: {
payload: column.id,
type: MessageFromWebviewType.OPEN_PARAMS_FILE_TO_THE_SIDE
}
}
]

return menuOptions
}, [column])

return (
<TableHeaderCell
column={column}
Expand All @@ -206,28 +232,21 @@ export const TableHeader: React.FC<TableHeaderProps> = ({
onDragOver={onDragOver}
onDragStart={onDragStart}
onDrop={onDrop}
menuDisabled={!isSortable}
menuDisabled={!isSortable && column.group !== ColumnType.PARAMS}
menuContent={
<div>
<SortPicker
sortOrder={sortOrder}
setSelectedOrder={order => {
setColumnSort(order)
}}
/>
<VSCodeDivider />
<MessagesMenu
options={[
{
id: 'hide-column',
label: 'Hide Column',
message: {
payload: column.id,
type: MessageFromWebviewType.HIDE_EXPERIMENTS_TABLE_COLUMN
}
}
]}
/>
{isSortable && (
<div>
<SortPicker
sortOrder={sortOrder}
setSelectedOrder={order => {
setColumnSort(order)
}}
/>
<VSCodeDivider />
</div>
)}
<MessagesMenu options={contextMenuOptions} />
</div>
}
/>
Expand Down
8 changes: 5 additions & 3 deletions webview/src/shared/components/messagesMenu/MessagesMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ export interface MessagesMenuProps {

export const MessagesMenu: React.FC<MessagesMenuProps> = ({ options }) => (
<div role="menu">
{options.map((option, i) => (
<MessagesMenuOption key={option.id} {...option} index={i} />
))}
{options
.filter(({ hidden }) => !hidden)
.map((option, i) => (
<MessagesMenuOption key={option.id} {...option} index={i} />
))}
</div>
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export interface MessagesMenuOptionProps {
id: string
label: string
message: MessageFromWebview
hidden?: boolean
}

interface MessagesMenuOptionAllProps extends MessagesMenuOptionProps {
Expand Down
2 changes: 2 additions & 0 deletions webview/src/util/array.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
export const pushIf = <T>(array: T[], condition: boolean, elements: T[]) =>
condition && array.push(...elements)