Merge pull request #876 from ethereum/ImproveAnnotation

Improve annotations

apps/remix-ide-e2e/src/commands/checkAnnotations.ts

@@ -0,0 +1,11 @@
+import EventEmitter from 'events'
+import { NightwatchBrowser } from 'nightwatch'
+
+class checkAnnotations extends EventEmitter {
+  command (this: NightwatchBrowser, type: string, line: number): NightwatchBrowser {
+    this.api.assert.containsText(`.ace_${type}`, line.toString()).perform(() => this.emit('complete'))
+    return this
+  }
+}
+
+module.exports = checkAnnotations

apps/remix-ide-e2e/src/commands/checkAnnotationsNotPresent.ts

@@ -0,0 +1,11 @@
+import EventEmitter from 'events'
+import { NightwatchBrowser } from 'nightwatch'
+
+class checkAnnotationsNotPresent extends EventEmitter {
+  command (this: NightwatchBrowser, type: string): NightwatchBrowser {
+    this.api.waitForElementNotPresent(`.ace_${type}`).perform(() => this.emit('complete'))
+    return this
+  }
+}
+
+module.exports = checkAnnotationsNotPresent

apps/remix-ide-e2e/src/tests/editor.test.ts

@@ -37,6 +37,11 @@ module.exports = {
       .sendKeys('*[class="ace_text-input"]', 'error')
       .pause(2000)
       .waitForElementVisible('.ace_error')
+      .checkAnnotations('error', 28)
+      .clickLaunchIcon('udapp')
+      .checkAnnotationsNotPresent('error')
+      .clickLaunchIcon('solidity')
+      .checkAnnotations('error', 28)
   },
 
   'Should minimize and maximize codeblock in editor': function (browser: NightwatchBrowser) {

apps/remix-ide-e2e/src/types/index.d.ts

@@ -53,6 +53,8 @@ declare module "nightwatch" {
         checkTerminalFilter(filter: string, test: string): NightwatchBrowser,
         noWorkerErrorFor(version: string): NightwatchBrowser,
         validateValueInput(selector: string, valueTosSet: string, expectedValue: string): NightwatchBrowser
+        checkAnnotations(type: string, line: number): NightwatchBrowser
+        checkAnnotationsNotPresent(type: string): NightwatchBrowser
     }
 
     export interface NightwatchBrowser {

apps/remix-ide/src/app.js

@@ -32,7 +32,6 @@ var GistHandler = require('./lib/gist-handler')
 var Storage = remixLib.Storage
 var RemixDProvider = require('./app/files/remixDProvider')
 var Config = require('./config')
-var Renderer = require('./app/ui/renderer')
 var examples = require('./app/editor/examples')
 var modalDialogCustom = require('./app/ui/modal-dialog-custom')
 var FileManager = require('./app/files/fileManager')
@@ -352,7 +351,6 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
   const compileTab = new CompileTab(
     editor,
     registry.get('config').api,
-    new Renderer(),
     registry.get('fileproviders/browser').api,
     registry.get('filemanager').api,
     contentImport
@@ -377,7 +375,6 @@ Please make a backup of your contracts and start using http://remix.ethereum.org
     filePanel,
     compileTab,
     appManager,
-    new Renderer(),
     contentImport
   )
 

apps/remix-ide/src/app/editor/editor.js

@@ -74,7 +74,7 @@ class Editor extends Plugin {
     // Init
     this.event = new EventManager()
     this.sessions = {}
-    this.sourceAnnotations = []
+    this.sourceAnnotationsPerFile = []
     this.readOnlySessions = {}
     this.previousInput = ''
     this.saveTimeout = null
@@ -203,8 +203,14 @@ class Editor extends Plugin {
   }
 
   onActivation () {
-    this.on('sidePanel', 'focusChanged', (name) => this.sourceHighlighters.hideHighlightsExcept(name))
+    this.on('sidePanel', 'focusChanged', (name) => {
-    this.on('sidePanel', 'pluginDisabled', (name) => this.sourceHighlighters.discardHighlight(name))
+      this.sourceHighlighters.hideHighlightsExcept(name)
+      this.keepAnnotationsFor(name)
+    })
+    this.on('sidePanel', 'pluginDisabled', (name) => {
+      this.sourceHighlighters.discardHighlight(name)
+      this.clearAllAnnotationsFor(name)
+    })
   }
 
   onDeactivation () {
@@ -499,28 +505,100 @@ class Editor extends Plugin {
   }
 
   /**
-   * Clears all the annotations for the current session.
+   * Clears all the annotations for the given @arg filePath and @arg plugin, if none is given, the current sesssion is used.
+   * An annotation has the following shape:
+      column: -1
+      row: -1
+      text: "browser/Untitled1.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.↵"
+      type: "warning"
+   * @param {String} filePath
+   * @param {String} plugin
    */
-  clearAnnotations () {
+  clearAnnotationsByPlugin (filePath, plugin) {
-    this.sourceAnnotations = []
+    if (filePath && !this.sessions[filePath]) throw new Error('file not found' + filePath)
-    this.editor.getSession().clearAnnotations()
+    const session = this.sessions[filePath] || this.editor.getSession()
+    const path = filePath || this.currentSession
+
+    const currentAnnotations = this.sourceAnnotationsPerFile[path]
+    if (!currentAnnotations) return
+
+    const newAnnotations = []
+    for (const annotation of currentAnnotations) {
+      if (annotation.from !== plugin) newAnnotations.push(annotation)
+    }
+    this.sourceAnnotationsPerFile[path] = newAnnotations
+
+    this._setAnnotations(session, path)
+  }
+
+  keepAnnotationsFor (name) {
+    if (!this.currentSession) return
+    if (!this.sourceAnnotationsPerFile[this.currentSession]) return
+
+    const annotations = this.sourceAnnotationsPerFile[this.currentSession]
+    for (const annotation of annotations) {
+      annotation.hide = annotation.from !== name
+    }
+
+    this._setAnnotations(this.editor.getSession(), this.currentSession)
   }
 
   /**
-   * Add an annotation to the current session.
+   * Clears all the annotations for the given @arg filePath, the plugin name is retrieved from the context, if none is given, the current sesssion is used.
-   * @param {Object} annotation
+   * An annotation has the following shape:
+      column: -1
+      row: -1
+      text: "browser/Untitled1.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.↵"
+      type: "warning"
+   * @param {String} filePath
+   * @param {String} plugin
    */
-  addAnnotation (annotation) {
+  clearAnnotations (filePath) {
-    this.sourceAnnotations[this.sourceAnnotations.length] = annotation
+    const { from } = this.currentRequest
-    this.setAnnotations(this.sourceAnnotations)
+    this.clearAnnotationsByPlugin(filePath, from)
   }
 
   /**
-   * Set a list of annotations to the current session.
+   * Clears all the annotations and for all the sessions for the given @arg plugin
-   * @param {Array<Object>} annotation
+   * An annotation has the following shape:
+      column: -1
+      row: -1
+      text: "browser/Untitled1.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.↵"
+      type: "warning"
+   * @param {String} filePath
    */
-  setAnnotations (sourceAnnotations) {
+  clearAllAnnotationsFor (plugin) {
-    this.editor.getSession().setAnnotations(sourceAnnotations)
+    for (const session in this.sessions) {
+      this.clearAnnotationsByPlugin(session, plugin)
+    }
+  }
+
+  /**
+   * Add an annotation to the current session.
+   * An annotation has the following shape:
+      column: -1
+      row: -1
+      text: "browser/Untitled1.sol: Warning: SPDX license identifier not provided in source file. Before publishing, consider adding a comment containing "SPDX-License-Identifier: <SPDX-License>" to each source file. Use "SPDX-License-Identifier: UNLICENSED" for non-open-source code. Please see https://spdx.org for more information.↵"
+      type: "warning"
+   * @param {Object} annotation
+   * @param {String} filePath
+   */
+  addAnnotation (annotation, filePath) {
+    if (filePath && !this.sessions[filePath]) throw new Error('file not found' + filePath)
+    const session = this.sessions[filePath] || this.editor.getSession()
+    const path = filePath || this.currentSession
+
+    const { from } = this.currentRequest
+    if (!this.sourceAnnotationsPerFile[path]) this.sourceAnnotationsPerFile[path] = []
+    annotation.from = from
+    this.sourceAnnotationsPerFile[path].push(annotation)
+
+    this._setAnnotations(session, path)
+  }
+
+  _setAnnotations (session, path) {
+    const annotations = this.sourceAnnotationsPerFile[path]
+    session.setAnnotations(annotations.filter((element) => !element.hide))
   }
 
   /**

apps/remix-ide/src/app/tabs/compile-tab.js

@@ -15,6 +15,7 @@ const copyToClipboard = require('../ui/copy-to-clipboard')
 const modalDialogCustom = require('../ui/modal-dialog-custom')
 const parseContracts = require('./compileTab/contractParser')
 const addTooltip = require('../ui/tooltip')
+const Renderer = require('../ui/renderer')
 const globalRegistry = require('../../global/registry')
 
 var css = require('./styles/compile-tab-styles')
@@ -41,7 +42,7 @@ const profile = {
 // - methods: ['getCompilationResult']
 
 class CompileTab extends ViewPlugin {
-  constructor (editor, config, renderer, fileProvider, fileManager, contentImport) {
+  constructor (editor, config, fileProvider, fileManager, contentImport) {
     super(profile)
     this.events = new EventEmitter()
     this._view = {
@@ -56,7 +57,7 @@ class CompileTab extends ViewPlugin {
     // dependencies
     this.editor = editor
     this.config = config
-    this.renderer = renderer
+    this.renderer = new Renderer(this)
     this.fileManager = fileManager
 
     this.data = {
@@ -67,7 +68,12 @@ class CompileTab extends ViewPlugin {
   }
 
   onActivationInternal () {
-    this.compileTabLogic = new CompileTabLogic(this.queryParams, this.fileManager, this.editor, this.config, this.fileProvider, this.contentImport)
+    const miscApi = {
+      clearAnnotations: () => {
+        this.call('editor', 'clearAnnotations')
+      }
+    }
+    this.compileTabLogic = new CompileTabLogic(this.queryParams, this.fileManager, this.editor, this.config, this.fileProvider, this.contentImport, miscApi)
     this.compiler = this.compileTabLogic.compiler
     this.compileTabLogic.init()
 

apps/remix-ide/src/app/tabs/compileTab/compileTab.js

@@ -2,8 +2,9 @@ const EventEmitter = require('events')
 var Compiler = require('@remix-project/remix-solidity').Compiler
 
 class CompileTab {
-  constructor (queryParams, fileManager, editor, config, fileProvider, contentImport) {
+  constructor (queryParams, fileManager, editor, config, fileProvider, contentImport, miscApi) {
     this.event = new EventEmitter()
+    this.miscApi = miscApi
     this.queryParams = queryParams
     this.compilerImport = contentImport
     this.compiler = new Compiler((url, cb) => this.compilerImport.resolveAndSave(url).then((result) => cb(null, result)).catch((error) => cb(error.message)))
@@ -80,7 +81,7 @@ class CompileTab {
   runCompiler () {
     try {
       this.fileManager.saveCurrentFile()
-      this.editor.clearAnnotations()
+      this.miscApi.clearAnnotations()
       var currentFile = this.config.get('currentFile')
       return this.compileFile(currentFile)
     } catch (err) {

apps/remix-ide/src/app/tabs/staticanalysis/staticAnalysisView.js

@@ -22,7 +22,7 @@ function staticAnalysisView (localRegistry, analysisModule) {
   this.sourceHighlighter = new SourceHighlighter()
   this.analysisModule = analysisModule
   self._components = {
-    renderer: new Renderer()
+    renderer: new Renderer(analysisModule)
   }
   self._components.registry = localRegistry
   // dependencies

apps/remix-ide/src/app/tabs/test-tab.js

@@ -3,6 +3,7 @@ import { canUseWorker, urlFromVersion } from '../compiler/compiler-utils'
 var yo = require('yo-yo')
 var async = require('async')
 var tooltip = require('../ui/tooltip')
+var Renderer = require('../ui/renderer')
 var css = require('./styles/test-tab-styles')
 var remixTests = require('@remix-project/remix-tests')
 
@@ -20,7 +21,7 @@ const profile = {
 }
 
 module.exports = class TestTab extends ViewPlugin {
-  constructor (fileManager, offsetToLineColumnConverter, filePanel, compileTab, appManager, renderer, contentImport) {
+  constructor (fileManager, offsetToLineColumnConverter, filePanel, compileTab, appManager, contentImport) {
     super(profile)
     this.compileTab = compileTab
     this.contentImport = contentImport
@@ -29,7 +30,7 @@ module.exports = class TestTab extends ViewPlugin {
     this.filePanel = filePanel
     this.data = {}
     this.appManager = appManager
-    this.renderer = renderer
+    this.renderer = new Renderer(this)
     this.hasBeenStopped = false
     this.runningTestsNumber = 0
     this.readyTestsNumber = 0

apps/remix-ide/src/app/ui/renderer.js

@@ -10,10 +10,11 @@ var globlalRegistry = require('../../global/registry')
  * TODO: This don't need to be an object anymore. Simplify and just export the renderError function.
  *
  */
-function Renderer (localRegistry) {
+function Renderer (service) {
   const self = this
+  self.service = service
   self._components = {}
-  self._components.registry = localRegistry || globlalRegistry
+  self._components.registry = globlalRegistry
   // dependencies
   self._deps = {
     fileManager: self._components.registry.get('filemanager').api,
@@ -26,9 +27,8 @@ function Renderer (localRegistry) {
 
 Renderer.prototype._error = function (file, error) {
   const self = this
-  const editor = self._components.registry.get('editor').api
   if (file === self._deps.config.get('currentFile')) {
-    editor.addAnnotation(error)
+    self.service.call('editor', 'addAnnotation', error, file)
   }
 }