Merge pull request #1070 from ethereum/fixFileExist

Fix file exist (from fileProvider)

ci/browser_tests.sh

@@ -30,11 +30,11 @@ while [ ! -f "$SAUCECONNECT_READYFILE" ]; do
   sleep .5
 done
 
-# npm run nightwatch_remote_chrome || TEST_EXITCODE=1
-# npm run nightwatch_remote_firefox || TEST_EXITCODE=1
-# npm run nightwatch_remote_safari || TEST_EXITCODE=1
+npm run nightwatch_remote_chrome || TEST_EXITCODE=1
+npm run nightwatch_remote_firefox || TEST_EXITCODE=1
+npm run nightwatch_remote_safari || TEST_EXITCODE=1
 # npm run nightwatch_remote_ie || TEST_EXITCODE=1
-npm run nightwatch_remote_parallel || TEST_EXITCODE=1
+# npm run nightwatch_remote_parallel || TEST_EXITCODE=1
 
 node ci/sauceDisconnect.js "$SAUCECONNECT_USERNAME" "$SAUCECONNECT_ACCESSKEY" "$SAUCECONNECT_JOBIDENTIFIER"
 

contracts/src/gmbh/company.sol

@@ -1,5 +1,6 @@
+import "./contract.sol";
 contract Assets {
-
+    uint[] proposals;
     function add(uint8 _numProposals) {
         proposals.length = _numProposals;
     }

contracts/src/gmbh/contract.sol

@@ -1,5 +1,5 @@
 contract gmbh {
-
+    uint[] proposals;
     function register(uint8 _numProposals) {
         proposals.length = _numProposals;
     }

src/app.js

@@ -3,6 +3,7 @@
 var $ = require('jquery')
 var csjs = require('csjs-inject')
 var yo = require('yo-yo')
+var async = require('async')
 var remixLib = require('remix-lib')
 var EventManager = remixLib.EventManager
 
@@ -214,10 +215,17 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
   // ----------------- Compiler -----------------
   var compiler = new Compiler((url, cb) => {
     var provider = fileManager.fileProviderOf(url)
-    if (provider && provider.exists(url)) {
-      return provider.get(url, cb)
-    }
-    handleImports.import(url,
+    if (provider) {
+      provider.exists(url, (error, exist) => {
+        if (error) return cb(error)
+        if (exist) {
+          return provider.get(url, cb)
+        } else {
+          return cb('Unable to import "' + url + '": File not found')
+        }
+      })
+    } else {
+      handleImports.import(url,
       (loadingMsg) => {
         $('#output').append($('<div/>').append($('<pre/>').text(loadingMsg)))
       },
@@ -229,6 +237,7 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
           cb(error)
         }
       })
+    }
   })
   var offsetToLineColumnConverter = new OffsetToLineColumnConverter(compiler.event)
 
@@ -386,14 +395,26 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
   this._components.contextView = new ContextView({
     contextualListener: this._components.contextualListener,
     jumpTo: (position) => {
+      function jumpToLine (lineColumn) {
+        if (lineColumn.start && lineColumn.start.line && lineColumn.start.column) {
+          editor.gotoLine(lineColumn.start.line, lineColumn.end.column + 1)
+        }
+      }
       if (compiler.lastCompilationResult && compiler.lastCompilationResult.data) {
         var lineColumn = offsetToLineColumnConverter.offsetToLineColumn(position, position.file, compiler.lastCompilationResult)
         var filename = compiler.getSourceName(position.file)
-        if (filename !== config.get('currentFile') && (filesProviders['browser'].exists(filename) || filesProviders['localhost'].exists(filename))) {
-          fileManager.switchFile(filename)
-        }
-        if (lineColumn.start && lineColumn.start.line && lineColumn.start.column) {
-          editor.gotoLine(lineColumn.start.line, lineColumn.end.column + 1)
+        // TODO: refactor with rendererAPI.errorClick
+        if (filename !== config.get('currentFile')) {
+          var provider = fileManager.fileProviderOf(filename)
+          if (provider) {
+            provider.exists(filename, (error, exist) => {
+              if (error) return console.log(error)
+              fileManager.switchFile(filename)
+              jumpToLine(lineColumn)
+            })
+          }
+        } else {
+          jumpToLine(lineColumn)
         }
       }
     }
@@ -443,15 +464,21 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
   function loadFiles (filesSet, fileProvider) {
     if (!fileProvider) fileProvider = 'browser'
 
-    for (var f in filesSet) {
-      var name = helper.createNonClashingName(f, filesProviders[fileProvider])
-      if (helper.checkSpecialChars(name)) {
-        modalDialogCustom.alert('Special characters are not allowed')
-        return
-      }
-      filesProviders[fileProvider].set(name, filesSet[f].content)
-    }
-    fileManager.switchFile()
+    async.each(Object.keys(filesSet), (file, callback) => {
+      helper.createNonClashingName(file, filesProviders[fileProvider],
+      (error, name) => {
+        if (error) {
+          modalDialogCustom.alert('Unexpected error loading the file ' + error)
+        } else if (helper.checkSpecialChars(name)) {
+          modalDialogCustom.alert('Special characters are not allowed')
+        } else {
+          filesProviders[fileProvider].set(name, filesSet[file].content)
+        }
+        callback()
+      })
+    }, (error) => {
+      if (!error) fileManager.switchFile()
+    })
   }
 
   // Replace early callback with instant response
@@ -541,10 +568,19 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
       }
     },
     errorClick: (errFile, errLine, errCol) => {
-      if (errFile !== config.get('currentFile') && (filesProviders['browser'].exists(errFile) || filesProviders['localhost'].exists(errFile))) {
-        fileManager.switchFile(errFile)
+      if (errFile !== config.get('currentFile')) {
+        // TODO: refactor with this._components.contextView.jumpTo
+        var provider = fileManager.fileProviderOf(errFile)
+        if (provider) {
+          provider.exists(errFile, (error, exist) => {
+            if (error) return console.log(error)
+            fileManager.switchFile(errFile)
+            editor.gotoLine(errLine, errCol)
+          })
+        }
+      } else {
+        editor.gotoLine(errLine, errCol)
       }
-      editor.gotoLine(errLine, errCol)
     }
   }
   var renderer = new Renderer(rendererAPI)

src/app/files/basicReadOnlyExplorer.js

@@ -21,10 +21,10 @@ class BasicReadOnlyExplorer {
     this.files = {}
   }
 
-  exists (path) {
-    if (!this.files) return false
+  exists (path, cb) {
+    if (!this.files) return cb(null, false)
     var unprefixedPath = this.removePrefix(path)
-    return this.files[unprefixedPath] !== undefined
+    cb(null, this.files[unprefixedPath] !== undefined)
   }
 
   get (path, cb) {

src/app/files/browser-files.js

@@ -8,7 +8,11 @@ function Files (storage) {
   var readonly = {}
   this.type = 'browser'
 
-  this.exists = function (path) {
+  this.exists = function (path, cb) {
+    cb(null, this._exists(path))
+  }
+
+  this._exists = function (path) {
     var unprefixedpath = this.removePrefix(path)
     // NOTE: ignore the config file
     if (path === '.remix.config') return false
@@ -75,7 +79,7 @@ function Files (storage) {
 
   this.remove = function (path) {
     var unprefixedpath = this.removePrefix(path)
-    if (!this.exists(unprefixedpath)) {
+    if (!this._exists(unprefixedpath)) {
       return false
     }
 
@@ -133,7 +137,7 @@ function Files (storage) {
   }
 
   // rename .browser-solidity.json to .remix.config
-  if (this.exists('.browser-solidity.json')) {
+  if (this._exists('.browser-solidity.json')) {
     this.rename('.browser-solidity.json', '.remix.config')
   }
 }

src/app/files/file-explorer.js

@@ -221,16 +221,21 @@ function fileExplorer (appAPI, files) {
       } else if (helper.checkSpecialChars(label.innerText)) {
         modalDialogCustom.alert('Special characters are not allowed')
         label.innerText = textUnderEdit
-      } else if (!files.exists(newPath)) {
-        files.rename(label.dataset.path, newPath, isFolder)
       } else {
-        modalDialogCustom.alert('File already exists.')
-        label.innerText = textUnderEdit
+        files.exists(newPath, (error, exist) => {
+          if (error) return modalDialogCustom.alert('Unexpected error while renaming: ' + error)
+          if (!exist) {
+            files.rename(label.dataset.path, newPath, isFolder)
+          } else {
+            modalDialogCustom.alert('File already exists.')
+            label.innerText = textUnderEdit
+          }
+        })
       }
     }
 
     if (event.which === 13) event.preventDefault()
-    if (event.type === 'blur' || event.which === 13 && label.getAttribute('contenteditable')) {
+    if ((event.type === 'blur' || event.which === 13) && label.getAttribute('contenteditable')) {
       var isFolder = label.className.indexOf('folder') !== -1
       var save = textUnderEdit !== label.innerText
       if (save) {

src/app/files/shared-folder.js

@@ -65,13 +65,11 @@ module.exports = class SharedFolder {
   //
   // this.remixd.exists(path, (error, isValid) => {})
 
-  exists (path) {
-    // @TODO: add new remixd.exists() method
-    // we remove the this.files = null at the beggining
-    // modify the exists() (cause it is using the this.files) to use remixd
-    // yes for the exists I think you might need another remixd function
-    if (!this.files) return false
-    return this.files[path] !== undefined
+  exists (path, cb) {
+    var unprefixedpath = this.removePrefix(path)
+    this._remixd.call('sharedfolder', 'exists', {path: unprefixedpath}, (error, result) => {
+      cb(error, result)
+    })
   }
 
   get (path, cb) {

src/app/panels/file-panel.js

@@ -169,17 +169,20 @@ function filepanel (appAPI, filesProvider) {
           }
           var success = files.set(name, event.target.result)
           if (!success) modalDialogCustom.alert('Failed to create file ' + name)
-          else self.events.trigger('focus', [name])
+          else self.event.trigger('focus', [name])
         }
         fileReader.readAsText(file)
       }
 
       var name = files.type + '/' + file.name
-      if (!files.exists(name)) {
-        loadFile()
-      } else {
-        modalDialogCustom.confirm(null, `The file ${name} already exists! Would you like to overwrite it?`, () => { loadFile() })
-      }
+      files.exists(name, (error, exist) => {
+        if (error) console.log(error)
+        if (!exist) {
+          loadFile()
+        } else {
+          modalDialogCustom.confirm(null, `The file ${name} already exists! Would you like to overwrite it?`, () => { loadFile() })
+        }
+      })
     })
   }
 
@@ -221,12 +224,14 @@ function filepanel (appAPI, filesProvider) {
 
   function createNewFile () {
     modalDialogCustom.prompt(null, 'File Name', 'Untitled.sol', (input) => {
-      var newName = filesProvider['browser'].type + '/' + helper.createNonClashingName(input, filesProvider['browser'])
-      if (!filesProvider['browser'].set(newName, '')) {
-        modalDialogCustom.alert('Failed to create file ' + newName)
-      } else {
-        appAPI.switchFile(newName)
-      }
+      helper.createNonClashingName(input, filesProvider['browser'], (error, newName) => {
+        if (error) return modalDialogCustom.alert('Failed to create file ' + newName + ' ' + error)
+        if (!filesProvider['browser'].set(newName, '')) {
+          modalDialogCustom.alert('Failed to create file ' + newName)
+        } else {
+          appAPI.switchFile(filesProvider['browser'].type + '/' + newName)
+        }
+      })
     })
   }
 

src/app/tabs/run-tab.js

@@ -161,12 +161,14 @@ function makeRecorder (events, appAPI, appEvents) {
       var fileProvider = appAPI.fileProviderOf(path)
       if (fileProvider) {
         var newFile = path + input
-        newFile = helper.createNonClashingName(newFile, fileProvider, '.json')
-        if (!fileProvider.set(newFile, txJSON)) {
-          modalDialogCustom.alert('Failed to create file ' + newFile)
-        } else {
-          appAPI.switchFile(newFile)
-        }
+        helper.createNonClashingName(newFile, fileProvider, (error, newFile) => {
+          if (error) return modalDialogCustom.alert('Failed to create file. ' + newFile + ' ' + error)
+          if (!fileProvider.set(newFile, txJSON)) {
+            modalDialogCustom.alert('Failed to create file ' + newFile)
+          } else {
+            appAPI.switchFile(newFile)
+          }
+        })
       }
     })
   }

src/app/ui/contextMenu.js

@@ -45,7 +45,7 @@ module.exports = (event, items) => {
   event.preventDefault()
 
   function hide (event, force) {
-    if (force || (event.target !== container)) {
+    if (container && container.parentElement && (force || (event.target !== container))) {
       container.parentElement.removeChild(container)
     }
     window.removeEventListener('click', hide)

src/lib/helper.js

@@ -1,3 +1,5 @@
+var async = require('async')
+
 module.exports = {
   shortenAddress: function (address, etherBalance) {
     var len = address.length
@@ -9,7 +11,7 @@ module.exports = {
     var len = data.length
     return data.slice(0, 5) + '...' + data.slice(len - 5, len)
   },
-  createNonClashingName (name, fileProvider) {
+  createNonClashingName (name, fileProvider, cb) {
     var counter = ''
     var ext = 'sol'
     var reg = /(.*)\.([^.]+)/g
@@ -18,10 +20,22 @@ module.exports = {
       name = split[1]
       ext = split[2]
     }
-    while (fileProvider.exists(name + counter + '.' + ext)) {
-      counter = (counter | 0) + 1
-    }
-    return name + counter + '.' + ext
+    var exist = true
+    async.whilst(
+      () => { return exist },
+      (callback) => {
+        fileProvider.exists(name + counter + '.' + ext, (error, currentExist) => {
+          if (error) {
+            callback(error)
+          } else {
+            exist = currentExist
+            if (exist) counter = (counter | 0) + 1
+            callback()
+          }
+        })
+      },
+      (error) => { cb(error, name + counter + '.' + ext) }
+    )
   },
   checkSpecialChars (name) {
     return name.match(/[/:*?"<>\\'|]/) != null

test-browser/tests/sharedFolderExplorer.js

@@ -3,9 +3,33 @@ var contractHelper = require('../helpers/contracts')
 var init = require('../helpers/init')
 var sauce = require('./sauce')
 
+var assetsTestContract = `import "./contract.sol";
+contract Assets {
+    uint[] proposals;
+    function add(uint8 _numProposals) {
+        proposals.length = _numProposals;
+    }
+}
+`
+
+var gmbhTestContract = `
+contract gmbh {
+    uint[] proposals;
+    function register(uint8 _numProposals) {
+        proposals.length = _numProposals;
+    }
+}
+`
 var sources = [
   {
     'localhost/folder1/contract2.sol': {content: 'contract test2 { function get () returns (uint) { return 11; }}'}
+  },
+  {
+    'localhost/src/gmbh/company.sol': {content: assetsTestContract}
+  },
+  {
+    'localhost/src/gmbh/company.sol': {content: assetsTestContract},
+    'localhost/src/gmbh/contract.sol': {content: gmbhTestContract}
   }
 ]
 
@@ -92,6 +116,9 @@ function runTests (browser, testData) {
         done()
       })
     })
+    .perform(function (done) {
+      testImportFromRemixd(browser, () => { done() })
+    })
     .perform(function () {
       browser.click('[data-path="localhost"]') // collapse and expand
         .waitForElementNotVisible('[data-path="localhost/folder1"]')
@@ -107,3 +134,19 @@ function runTests (browser, testData) {
         .end()
     })
 }
+
+function testImportFromRemixd (browser, callback) {
+  browser
+    .waitForElementVisible('[data-path="localhost/src"]', 100000)
+    .click('[data-path="localhost/src"]')
+    .waitForElementVisible('[data-path="localhost/src/gmbh"]', 100000)
+    .click('[data-path="localhost/src/gmbh"]')
+    .waitForElementVisible('[data-path="localhost/src/gmbh/company.sol"]', 100000)
+    .click('[data-path="localhost/src/gmbh/company.sol"]')
+    .pause(1000)
+    .perform(() => {
+      contractHelper.verifyContract(browser, ['Assets', 'gmbh'], function () {
+        callback()
+      })
+    })
+}