Fix plots paths breaking on windows (#4607)

* Fix plots paths breaking on windows

* Standardise plots output paths

* Add tests

* Apply review comments

Author: julieg18

extension/src/fileSystem/util.ts

@@ -2,6 +2,9 @@ import { sep, parse } from 'path'
 
 export const getPathArray = (path: string): string[] => path.split(sep)
 
+export const ensureOsFileSep = (path: string): string =>
+  path.replace(/\\/g, '/').split('/').join(sep)
+
 export const getPath = (pathArray: string[], idx: number) =>
   pathArray.slice(0, idx).join(sep)
 

extension/src/plots/index.ts

@@ -5,6 +5,7 @@ import { WebviewMessages } from './webview/messages'
 import { PlotsData } from './data'
 import { ErrorsModel } from './errors/model'
 import { PlotsModel } from './model'
+import { ensurePlotsDataPathsOsSep } from './util'
 import { collectEncodingElements, collectScale } from './paths/collect'
 import { PathsModel } from './paths/model'
 import { pickCustomPlots, pickMetricAndParam } from './model/quickPick'
@@ -292,10 +293,11 @@ export class Plots extends BaseRepository<TPlotsData> {
   private onDidUpdateData() {
     this.dispose.track(
       this.data.onDidUpdate(async ({ data, revs }) => {
+        const standardisedData = ensurePlotsDataPathsOsSep(data)
         await Promise.all([
-          this.plots.transformAndSet(data, revs),
-          this.paths.transformAndSet(data, revs),
-          this.errors.transformAndSet(data, revs)
+          this.plots.transformAndSet(standardisedData, revs),
+          this.paths.transformAndSet(standardisedData, revs),
+          this.errors.transformAndSet(standardisedData, revs)
         ])
         this.notifyChanged()
       })

extension/src/plots/util.test.ts

@@ -0,0 +1,58 @@
+import { sep } from 'path'
+import { ensurePlotsDataPathsOsSep } from './util'
+import { PlotsType } from './webview/contract'
+import { FIELD_SEPARATOR } from '../cli/dvc/constants'
+import { PlotsOutput } from '../cli/dvc/contract'
+
+const joinWithSep = (pathArr: string[], slash = sep) => pathArr.join(slash)
+
+const getOutput = (slash = sep): PlotsOutput => {
+  return {
+    data: {
+      [joinWithSep(['plots', 'heatmap.png'], slash)]: [
+        {
+          revisions: ['main'],
+          type: PlotsType.IMAGE,
+          url: joinWithSep(['plots', 'heatmap.png'])
+        }
+      ],
+      [joinWithSep([`dvc.yaml${FIELD_SEPARATOR}logs`, 'acc.tsv'], slash)]: [
+        {
+          content: {},
+          datapoints: { main: [{}] },
+          revisions: ['main'],
+          type: PlotsType.VEGA
+        }
+      ]
+    },
+    errors: [
+      {
+        msg: 'No such file or directory',
+        name: joinWithSep(['plots', 'heatmap.png'], slash),
+        rev: 'main',
+        type: 'FileNotFoundError'
+      }
+    ]
+  }
+}
+
+const windowsStyleOutput = getOutput('\\')
+const unixStyleOutput = getOutput('/')
+const osStyleOutput = getOutput()
+
+describe('ensurePlotsDataPathsOsSep', () => {
+  it('should update windows and unix style data paths to style based by os', () => {
+    expect(ensurePlotsDataPathsOsSep(windowsStyleOutput)).toStrictEqual(
+      osStyleOutput
+    )
+    expect(ensurePlotsDataPathsOsSep(unixStyleOutput)).toStrictEqual(
+      osStyleOutput
+    )
+  })
+  it('should return early if there is a cli error', () => {
+    const cliError = {
+      error: { msg: 'something has gone wrong', type: 'clierror' }
+    }
+    expect(ensurePlotsDataPathsOsSep(cliError)).toStrictEqual(cliError)
+  })
+})

extension/src/plots/util.ts

@@ -0,0 +1,30 @@
+import { PlotsOutput, PlotsOutputOrError } from '../cli/dvc/contract'
+import { isDvcError } from '../cli/dvc/reader'
+import { ensureOsFileSep } from '../fileSystem/util'
+
+export const ensurePlotsDataPathsOsSep = (
+  plotsData: PlotsOutputOrError
+): PlotsOutputOrError => {
+  if (isDvcError(plotsData)) {
+    return plotsData
+  }
+  const standardisedData: PlotsOutput = { data: {} }
+  const { data, errors } = plotsData
+
+  for (const path of Object.keys(data)) {
+    standardisedData.data[ensureOsFileSep(path)] = data[path]
+  }
+
+  if (!errors) {
+    return standardisedData
+  }
+  standardisedData.errors = errors.map(error => {
+    if (!error.name) {
+      return error
+    }
+
+    return { ...error, name: ensureOsFileSep(error.name) }
+  })
+
+  return standardisedData
+}