Skip to content

Commit

Permalink
refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
mattseddon committed Jul 17, 2023
1 parent 3c585c0 commit ed8cfad
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 24 deletions.
8 changes: 3 additions & 5 deletions extension/src/data/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { InternalCommands } from '../commands/internal'
import { ExpShowOutput, PlotsOutputOrError } from '../cli/dvc/contract'
import { uniqueValues } from '../util/array'
import { DeferredDisposable } from '../class/deferred'
import { isSameOrChild } from '../fileSystem'
import { isPathInSubProject, isSameOrChild } from '../fileSystem'

export type ExperimentsOutput = {
availableNbCommits: { [branch: string]: number }
Expand Down Expand Up @@ -86,14 +86,12 @@ export abstract class BaseData<
path => {
const relPath = relative(this.dvcRoot, path)
if (
!this.relSubProjects.some(subProject =>
relPath.startsWith(subProject)
) &&
this.getWatchedFiles().some(
watchedRelPath =>
path.endsWith(watchedRelPath) ||
isSameOrChild(relPath, watchedRelPath)
)
) &&
!isPathInSubProject(relPath, this.relSubProjects)
) {
void this.managedUpdate(path)
}
Expand Down
6 changes: 3 additions & 3 deletions extension/src/experiments/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Event, EventEmitter, window } from 'vscode'
import { Disposable, Disposer } from '@hediet/std/disposable'
import { ContextKey, setContextValue } from '../vscode/context'
import { standardizePossiblePath } from '../fileSystem/path'
import { isFileInSubProject } from '../fileSystem'
import { isPathInProject, isPathInSubProject } from '../fileSystem'

const setContextOnDidChangeParamsFiles = (
setActiveEditorContext: (paramsFileActive: boolean) => void,
Expand All @@ -20,7 +20,7 @@ const setContextOnDidChangeParamsFiles = (

if (
(!getParamsFiles().has(path) && !path.endsWith('dvc.yaml')) ||
isFileInSubProject(path, subProjects)
isPathInSubProject(path, subProjects)
) {
return
}
Expand All @@ -40,7 +40,7 @@ const setContextOnDidChangeActiveEditor = (
return
}

if (!path.includes(dvcRoot) || isFileInSubProject(path, subProjects)) {
if (!isPathInProject(path, dvcRoot, subProjects)) {
return
}

Expand Down
2 changes: 1 addition & 1 deletion extension/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,8 @@ class Extension extends Disposable {
public async initialize() {
this.resetMembers()

const subProjects = this.getSubProjects()
const dvcRoots = this.getRoots()
const subProjects = this.getSubProjects()

await Promise.all([
this.repositories.create(dvcRoots, subProjects),
Expand Down
51 changes: 50 additions & 1 deletion extension/src/fileSystem/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ import {
getModifiedTime,
findOrCreateDvcYamlFile,
writeJson,
writeCsv
writeCsv,
isPathInProject
} from '.'
import { dvcDemoPath } from '../test/util'
import { DOT_DVC } from '../cli/dvc/constants'
Expand Down Expand Up @@ -408,3 +409,51 @@ describe('findOrCreateDvcYamlFile', () => {
)
})
})

describe('isPathInProject', () => {
it('should return true if the path is in the project', () => {
const path = join(dvcDemoPath, 'dvc.yaml')
const dvcRoot = dvcDemoPath
const subProjects: string[] = []
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(true)
})
it('should return false if the path is not in the project', () => {
const path = resolve(dvcDemoPath, '..', 'dvc.yaml')
const dvcRoot = dvcDemoPath
const subProjects: string[] = []
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(false)
})

it('should return false if the path is the project', () => {
const path = dvcDemoPath
const dvcRoot = dvcDemoPath
const subProjects: string[] = []
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(false)
})

it('should return false if the path is in the project but also in a sub-project', () => {
const path = join(dvcDemoPath, 'nested1', 'dvc.yaml')
const dvcRoot = dvcDemoPath
const subProjects: string[] = [join(dvcDemoPath, 'nested1')]
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(false)
})

it('should return false if the path is in the project but also in one of many sub-projects', () => {
const path = join(dvcDemoPath, 'nested2', 'dvc.yaml')
const dvcRoot = dvcDemoPath
const subProjects: string[] = [
join(dvcDemoPath, 'nested1'),
join(dvcDemoPath, 'nested2'),
join(dvcDemoPath, 'nested3'),
join(dvcDemoPath, 'nested4')
]
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(false)
})

it('should return true if the path is in the project but not in a sub-project', () => {
const path = join(dvcDemoPath, 'nested1', 'dvc.yaml')
const dvcRoot = dvcDemoPath
const subProjects: string[] = [join(dvcDemoPath, 'nested2')]
expect(isPathInProject(path, dvcRoot, subProjects)).toBe(true)
})
})
15 changes: 12 additions & 3 deletions extension/src/fileSystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,19 @@ export const isSameOrChild = (root: string, path: string) => {
return !rel.startsWith('..')
}

export const isFileInSubProject = (
file: string,
export const isPathInSubProject = (
path: string,
subProjects: string[]
): boolean => subProjects.some(dvcRoot => path.startsWith(dvcRoot))

export const isPathInProject = (
path: string | undefined,
dvcRoot: string,
subProjects: string[]
): boolean => subProjects.some(dvcRoot => file.startsWith(dvcRoot))
): path is string =>
!!path?.startsWith(dvcRoot) &&
path !== dvcRoot &&
!isPathInSubProject(path, subProjects)

export type Out =
| string
Expand Down
9 changes: 3 additions & 6 deletions extension/src/pipeline/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { EventEmitter, window } from 'vscode'
import { Disposable, Disposer } from '@hediet/std/disposable'
import { ContextKey, setContextValue } from '../vscode/context'
import { standardizePossiblePath } from '../fileSystem/path'
import { isFileInSubProject } from '../fileSystem'
import { isPathInProject } from '../fileSystem'

const setContextOnDidChangeActiveEditor = (
setActiveEditorContext: (path: string) => void,
Expand All @@ -17,7 +17,7 @@ const setContextOnDidChangeActiveEditor = (
return
}

if (!path.includes(dvcRoot) || isFileInSubProject(path, subProjects)) {
if (!isPathInProject(path, dvcRoot, subProjects)) {
return
}

Expand All @@ -37,10 +37,7 @@ export const setContextForEditorTitleIcons = (
}

const activePath = window.activeTextEditor?.document?.fileName
if (
activePath?.startsWith(dvcRoot) &&
!isFileInSubProject(activePath, subProjects)
) {
if (isPathInProject(activePath, dvcRoot, subProjects)) {
setActiveEditorContext(activePath)
}

Expand Down
7 changes: 2 additions & 5 deletions extension/src/pipeline/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { dirname } from 'path'
import { AvailableCommands, InternalCommands } from '../commands/internal'
import { BaseData } from '../data'
import { findFiles } from '../fileSystem/workspace'
import { isFileInSubProject } from '../fileSystem'
import { isPathInProject } from '../fileSystem'

export class PipelineData extends BaseData<{
dag: string
Expand Down Expand Up @@ -38,10 +38,7 @@ export class PipelineData extends BaseData<{

const dvcYamlsDirs = new Set<string>()
for (const file of fileList) {
if (
file.startsWith(this.dvcRoot) &&
!isFileInSubProject(file, this.subProjects)
) {
if (isPathInProject(file, this.dvcRoot, this.subProjects)) {
dvcYamlsDirs.add(dirname(file))
}
}
Expand Down

0 comments on commit ed8cfad

Please sign in to comment.