Split commands in tracked explorer tree between virtual and non-virtu…

…al paths (#816) * add virtual context to tracked explorer tree * remove dead code * move registration into vscode commands * remove remaining reference to dead option
iterative · Sep 16, 2021 · f5fb1eb · f5fb1eb
1 parent ff916dc
commit f5fb1eb
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 154 deletions.
diff --git a/extension/package.json b/extension/package.json
@@ -280,12 +280,6 @@
           "description": "%config.pythonPath.description%",
           "type": "string",
           "default": null
-        },
-        "dvc.views.trackedExplorerTree.noPromptPullMissing": {
-          "title": "%config.noPromptPullMissing.title%",
-          "description": "%config.noPromptPullMissing.description%",
-          "type": "boolean",
-          "default": false
         }
       }
     },
@@ -503,22 +497,22 @@
         {
           "command": "dvc.pullTarget",
           "group": "1_DVC@1",
-          "when": "viewItem =~ /^dvcTracked/ && viewItem =~ /HasRemote$|Data$/"
+          "when": "viewItem =~ /^virtualDvcTracked|^dvcTracked.*[HasRemote|Data]$/"
         },
         {
           "command": "dvc.pushTarget",
           "group": "1_DVC@2",
-          "when": "viewItem =~ /^dvcTracked/ && viewItem =~ /HasRemote$|Data$/"
+          "when": "viewItem =~ /^dvcTracked.*[HasRemote|Data]$/"
         },
         {
           "command": "dvc.copyFilePath",
           "group": "6_copypath@1",
-          "when": "viewItem =~ /^dvcTracked/"
+          "when": "viewItem =~ /[d|D]vcTracked/"
         },
         {
           "command": "dvc.copyRelativeFilePath",
           "group": "6_copypath@2",
-          "when": "viewItem =~ /^dvcTracked/"
+          "when": "viewItem =~ /[d|D]vcTracked/"
         },
         {
           "command": "dvc.openToTheSide",
@@ -528,7 +522,7 @@
         {
           "command": "dvc.renameTarget",
           "group": "7_modification@1",
-          "when": "viewItem =~ /^dvcTracked/ && viewItem =~ /Data$/"
+          "when": "viewItem =~ /^dvcTracked.*Data$/"
         },
         {
           "command": "dvc.deleteTarget",
@@ -538,7 +532,7 @@
         {
           "command": "dvc.removeTarget",
           "group": "7_modification@2",
-          "when": "viewItem =~ /^dvcTracked/ && viewItem =~ /Data$/"
+          "when": "viewItem =~ /^dvcTracked.*Data$/"
         },
         {
           "command": "dvc.addExperimentsTableSort",

diff --git a/extension/src/fileSystem/tree.test.ts b/extension/src/fileSystem/tree.test.ts
@@ -108,13 +108,14 @@ describe('TrackedTreeView', () => {
   })
 
   describe('getTreeItem', () => {
-    it('should return the correct tree item for a directory', async () => {
+    it('should return the correct tree item for a virtual directory', async () => {
       let mockedItem = {}
       mockedTreeItem.mockImplementationOnce(function (uri, collapsibleState) {
         expect(collapsibleState).toEqual(1)
         mockedItem = { collapsibleState, uri }
         return mockedItem
       })
+      mockedExists.mockReturnValueOnce(false)
 
       mockedListDvcOnly.mockResolvedValueOnce(demoRootList)
 
@@ -131,7 +132,7 @@ describe('TrackedTreeView', () => {
       expect(mockedTreeItem).toBeCalledTimes(1)
       expect(treeItem).toEqual({
         ...mockedItem,
-        contextValue: 'dvcTracked'
+        contextValue: 'virtualDvcTracked'
       })
     })
 
@@ -150,7 +151,7 @@ describe('TrackedTreeView', () => {
         mockedTreeDataChanged
       )
       trackedTreeView.initialize([dvcDemoPath])
-      mockedExists.mockReturnValueOnce(true)
+      mockedExists.mockReturnValueOnce(true).mockReturnValueOnce(true)
 
       await trackedTreeView.getChildren()
 
@@ -179,6 +180,7 @@ describe('TrackedTreeView', () => {
         mockedWorkspaceChanged,
         mockedTreeDataChanged
       )
+      mockedExists.mockReturnValueOnce(true)
 
       const treeItem = trackedTreeView.getTreeItem(log)
 

diff --git a/extension/src/fileSystem/tree.ts b/extension/src/fileSystem/tree.ts
@@ -1,6 +1,5 @@
 import { dirname, join, relative } from 'path'
 import {
-  commands,
   Event,
   EventEmitter,
   TreeDataProvider,
@@ -14,7 +13,6 @@ import { exists } from '.'
 import { deleteTarget } from './workspace'
 import { definedAndNonEmpty } from '../util/array'
 import { ListOutput } from '../cli/reader'
-import { getConfigValue, setConfigValue } from '../vscode/config'
 import { tryThenMaybeForce } from '../cli/actions'
 import {
   CommandId,
@@ -41,11 +39,6 @@ export class TrackedExplorerTree implements TreeDataProvider<string> {
   private pathIsDirectory: Record<string, boolean> = {}
   private pathIsOut: Record<string, boolean> = {}
 
-  private doNotShowAgainText = "Don't Show Again"
-
-  private noPromptPullMissingOption =
-    'dvc.views.trackedExplorerTree.noPromptPullMissing'
-
   private viewed = false
 
   constructor(
@@ -116,25 +109,6 @@ export class TrackedExplorerTree implements TreeDataProvider<string> {
     return treeItem
   }
 
-  private openPullPrompt = async (path: string) => {
-    if (getConfigValue(this.noPromptPullMissingOption)) {
-      return
-    }
-    const response = await window.showInformationMessage(
-      `${path} does not exist at the specified path.`,
-      'Pull File',
-      this.doNotShowAgainText
-    )
-
-    if (response === 'Pull File') {
-      return this.tryThenMaybeForce(AvailableCommands.PULL, path)
-    }
-
-    if (response === this.doNotShowAgainText) {
-      return setConfigValue(this.noPromptPullMissingOption, true)
-    }
-  }
-
   private async getRootElements() {
     if (!this.viewed) {
       sendViewOpenedTelemetryEvent(
@@ -158,16 +132,6 @@ export class TrackedExplorerTree implements TreeDataProvider<string> {
       })
   }
 
-  private openResource = (resource: Uri, commandId: string) => {
-    const path = resource.fsPath
-
-    if (!exists(path)) {
-      return this.openPullPrompt(path)
-    }
-
-    return commands.executeCommand(commandId, resource)
-  }
-
   private getDataPlaceholder(path: string): string {
     return path.trim() + '.dvc'
   }
@@ -181,6 +145,10 @@ export class TrackedExplorerTree implements TreeDataProvider<string> {
   }
 
   private getContextValue(path: string): string {
+    if (!exists(path)) {
+      return 'virtualDvcTracked'
+    }
+
     const baseContext = this.pathIsDirectory[path]
       ? 'dvcTracked'
       : 'dvcTrackedFile'
@@ -230,16 +198,6 @@ export class TrackedExplorerTree implements TreeDataProvider<string> {
       }
     )
 
-    this.internalCommands.registerExternalCommand<Uri>(
-      RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
-      resource => this.openResource(resource, 'vscode.open')
-    )
-
-    this.internalCommands.registerExternalCommand<string>(
-      RegisteredCommands.TRACKED_EXPLORER_OPEN_TO_THE_SIDE,
-      path => this.openResource(Uri.file(path), 'explorer.openToSide')
-    )
-
     this.internalCommands.registerExternalCommand<string>(
       RegisteredCommands.DELETE_TARGET,
       path => deleteTarget(path)

diff --git a/extension/src/test/suite/fileSystem/tree.test.ts b/extension/src/test/suite/fileSystem/tree.test.ts
@@ -20,7 +20,6 @@ import {
   RegisteredCliCommands,
   RegisteredCommands
 } from '../../../commands/external'
-import { setConfigValue } from '../../../vscode/config'
 
 suite('Tracked Explorer Tree Test Suite', () => {
   window.showInformationMessage('Start all tracked explorer tree tests.')
@@ -35,10 +34,6 @@ suite('Tracked Explorer Tree Test Suite', () => {
 
   afterEach(() => {
     disposable.dispose()
-    setConfigValue(
-      'dvc.views.trackedExplorerTree.noPromptPullMissing',
-      undefined
-    )
     return commands.executeCommand('workbench.action.closeAllEditors')
   })
 
@@ -127,94 +122,6 @@ suite('Tracked Explorer Tree Test Suite', () => {
       expect(window.activeTextEditor?.viewColumn).not.to.equal(ViewColumn.One)
     })
 
-    it('should not fail to open a file to the side if it is not on disk', async () => {
-      const missingFile = 'missing.txt'
-      const mockShowInformationMessage = stub(
-        window,
-        'showInformationMessage'
-      ).resolves(undefined)
-
-      await commands.executeCommand(
-        RegisteredCommands.TRACKED_EXPLORER_OPEN_TO_THE_SIDE,
-        missingFile
-      )
-
-      expect(mockShowInformationMessage).to.be.calledOnce
-    })
-
-    it('should be able to pull a file after trying to open it when it does not exist on disk', async () => {
-      const missingFile = 'non-existent.txt'
-      const absPath = join(dvcDemoPath, missingFile)
-      const uri = Uri.file(absPath)
-      stub(path, 'relative').returns(missingFile)
-
-      const mockShowInformationMessage = stub(window, 'showInformationMessage')
-
-      mockShowInformationMessage.resolves(undefined)
-      const mockPull = stub(CliExecutor.prototype, 'pull').resolves(
-        'M       non-existent.txt\n1 file modified'
-      )
-
-      await commands.executeCommand(
-        RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
-        uri
-      )
-
-      expect(
-        mockShowInformationMessage,
-        'should show the user an information prompt'
-      ).to.be.calledOnce
-      expect(mockPull, 'should not call pull if the prompt is dismissed').not.to
-        .be.called
-
-      mockShowInformationMessage.resetHistory()
-      mockShowInformationMessage.resolves('Pull File' as unknown as MessageItem)
-
-      await commands.executeCommand(
-        RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
-        uri
-      )
-
-      expect(
-        mockShowInformationMessage,
-        'should show the user an information prompt'
-      ).to.be.calledOnce
-      expect(mockPull, 'should pull the file if prompted').to.be.calledOnce
-
-      mockPull.resetHistory()
-      mockShowInformationMessage.resetHistory()
-      mockShowInformationMessage.resolves(
-        "Don't Show Again" as unknown as MessageItem
-      )
-
-      await commands.executeCommand(
-        RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
-        uri
-      )
-
-      expect(
-        mockShowInformationMessage,
-        'should show the user an information prompt'
-      ).to.be.calledOnce
-      expect(
-        mockPull,
-        'should not pull the file if the user chooses do not show again'
-      ).not.to.be.called
-
-      mockShowInformationMessage.resetHistory()
-
-      await commands.executeCommand(
-        RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
-        uri
-      )
-
-      expect(
-        mockShowInformationMessage,
-        'should not show the information prompt if the appropriate config option is set'
-      ).not.to.be.called
-      expect(mockPull, 'should not pull the file').not.to.be.called
-    })
-
     it('should be able to run dvc.removeTarget without error', async () => {
       const relPath = join('mock', 'data', 'MNIST', 'raw')
       const absPath = join(dvcDemoPath, relPath)

diff --git a/extension/src/vscode/commands.ts b/extension/src/vscode/commands.ts
@@ -17,4 +17,14 @@ export const reRegisterVsCodeCommands = (
     RegisteredCommands.TRACKED_EXPLORER_COPY_REL_FILE_PATH,
     path => executeCommand('copyRelativeFilePath', path)
   )
+
+  internalCommands.registerExternalCommand<Uri>(
+    RegisteredCommands.TRACKED_EXPLORER_OPEN_FILE,
+    resource => commands.executeCommand('vscode.open', resource)
+  )
+
+  internalCommands.registerExternalCommand<string>(
+    RegisteredCommands.TRACKED_EXPLORER_OPEN_TO_THE_SIDE,
+    path => executeCommand('explorer.openToSide', path)
+  )
 }