WIP - #533 - Part 1: Error API Improvements & Refactoring (#534)

* Start refactoring interface to use diagnostic type

* Continue working with types.Diagnostic

* Refactor to use types.Diagnostic

* Update types

* Bring back setErrors call

* Restore ErrorOverlay functionality

* Fix error messages

* Restore error / warning behavior

* Add SetErrorsAction/ClearErrotsAction

* Add error info to state

* Add action creators for set / clear errors

* Hook up react store to provider

* Add reselect

* Reformat to use store data instead of separate array of errors

* Fix selector

* Add reducer to handle errors

* Add window state to redux store

* Create actions for window state

* Add reducer for window actions

* Set up windowState and hook up reducer to top-level reducer

* Add ActiveWindow component

* Test out ActiveWindowContainer

* Remove background color

* Create new, side-by-side implementation of WindowContext to support refactoring. WindowContext2 will replace WindowContext once the refactoring is complete.

* Remove ErrorOverlay, refactor to use store state completely

* Fix some runtime errors

* Fix path normalization and preserve existing errors

* Add windowTopLine/windowBottomLine to redux store, and move more state over to the connected component

* Add error markers

* Add cursor and errors back in

* Remove ScrollBarOverlay

* Add SET_BUFFER_STATE action

* Wire up total buffer lines

* Remove error test in NeovimInstance

* Remove legacy SetActiveWindowDimensions action

* Refactor ActiveWindow to separate file

* Fix issue with empty string buffer name

* Show details only when not in insert mode

* Fix regerssion with cursorLine

* Add PLAN, fix compile errors after merge

* Fix lint issues

* Remove PLAN.md

* Update failing unit tests

browser/src/Editor/NeovimEditor.tsx

@@ -4,11 +4,11 @@
  * IEditor implementation for Neovim
  */
 
-import * as path from "path"
-
 import * as React from "react"
 import * as ReactDOM from "react-dom"
 
+import * as types from "vscode-languageserver-types"
+
 import { ipcRenderer } from "electron"
 
 import { IncrementalDeltaRegionTracker } from "./../DeltaRegionTracker"
@@ -35,10 +35,8 @@ import { Tasks } from "./../Services/Tasks"
 import { WindowTitle } from "./../Services/WindowTitle"
 
 import * as UI from "./../UI/index"
-import { ErrorOverlay } from "./../UI/Overlay/ErrorOverlay"
 import { LiveEvaluationOverlay } from "./../UI/Overlay/LiveEvaluationOverlay"
 import { OverlayManager } from "./../UI/Overlay/OverlayManager"
-import { ScrollBarOverlay } from "./../UI/Overlay/ScrollBarOverlay"
 import { Rectangle } from "./../UI/Types"
 
 import { Keyboard } from "./../Input/Keyboard"
@@ -61,10 +59,8 @@ export class NeovimEditor implements IEditor {
     private _tasks: Tasks
 
     // Overlays
-    private _errorOverlay: ErrorOverlay
     private _overlayManager: OverlayManager
     private _liveEvaluationOverlay: LiveEvaluationOverlay
-    private _scrollbarOverlay: ScrollBarOverlay
 
     private _errorStartingNeovim: boolean = false
 
@@ -112,14 +108,12 @@ export class NeovimEditor implements IEditor {
         // TODO: Replace `OverlayManagement` concept and associated window management code with
         // explicit window management: #362
         this._overlayManager = new OverlayManager(this._screen, this._neovimInstance)
-        this._errorOverlay = new ErrorOverlay()
         this._liveEvaluationOverlay = new LiveEvaluationOverlay()
-        this._scrollbarOverlay = new ScrollBarOverlay()
-        this._overlayManager.addOverlay("errors", this._errorOverlay)
         this._overlayManager.addOverlay("live-eval", this._liveEvaluationOverlay)
-        this._overlayManager.addOverlay("scrollbar", this._scrollbarOverlay)
 
-        this._overlayManager.on("current-window-size-changed", (dimensionsInPixels: Rectangle) => UI.Actions.setActiveWindowDimensions(dimensionsInPixels))
+        this._overlayManager.on("current-window-size-changed", (dimensionsInPixels: Rectangle, windowId: number) => {
+            UI.Actions.setWindowDimensions(windowId, dimensionsInPixels)
+        })
 
         // TODO: Refactor `pluginManager` responsibilities outside of this instance
         this._pluginManager.on("signature-help-response", (err: string, signatureHelp: any) => { // FIXME: setup Oni import
@@ -130,18 +124,11 @@ export class NeovimEditor implements IEditor {
             }
         })
 
-        this._pluginManager.on("set-errors", (key: string, fileName: string, errors: any[], color: string) => {
-            errorService.setErrors(fileName, errors)
+        this._pluginManager.on("set-errors", (key: string, fileName: string, errors: types.Diagnostic[]) => {
 
-            color = color || "red"
-            this._errorOverlay.setErrors(key, fileName, errors, color)
+            UI.Actions.setErrors(fileName, key, errors)
 
-            const errorMarkers = errors.map((e: any) => ({
-                line: e.lineNumber,
-                height: 1,
-                color,
-            }))
-            this._scrollbarOverlay.setMarkers(path.resolve(fileName), key, errorMarkers)
+            errorService.setErrors(fileName, errors)
         })
 
         liveEvaluation.on("evaluate-block-result", (file: string, blocks: any[]) => {
@@ -170,14 +157,12 @@ export class NeovimEditor implements IEditor {
             ReactDOM.render(<InstallHelp />, this._element.parentElement)
         })
 
-        this._neovimInstance.on("buffer-update", (context: any, lines: string[]) => {
-            this._scrollbarOverlay.onBufferUpdate(context, lines)
-        })
-
         this._neovimInstance.on("window-display-update", (eventContext: Oni.EventContext, lineMapping: any, shouldMeasure: boolean) => {
             if (shouldMeasure) {
                 this._overlayManager.notifyWindowDimensionsChanged(eventContext, lineMapping)
             }
+
+            UI.Actions.setWindowLineMapping(eventContext.windowNumber, lineMapping)
         })
 
         this._neovimInstance.on("action", (action: any) => {
@@ -332,31 +317,28 @@ export class NeovimEditor implements IEditor {
         UI.Actions.setMode(newMode)
 
         if (newMode === "normal") {
+            UI.Actions.showCursorLine()
             UI.Actions.showCursorColumn()
             UI.Actions.hideCompletions()
             UI.Actions.hideSignatureHelp()
         } else if (newMode === "insert") {
             UI.Actions.hideQuickInfo()
             UI.Actions.showCursorColumn()
+            UI.Actions.showCursorLine()
         } else if (newMode.indexOf("cmdline") >= 0) {
+            UI.Actions.hideCursorLine()
             UI.Actions.hideCursorColumn() // TODO: cleaner way to hide and unhide?
             UI.Actions.hideCompletions()
             UI.Actions.hideQuickInfo()
         }
-
-        // Error overlay
-        if (newMode === "insert") {
-            this._errorOverlay.hideDetails()
-        } else {
-            this._errorOverlay.showDetails()
-        }
     }
 
-    private _onVimEvent(eventName: string, evt: any): void {
+    private _onVimEvent(eventName: string, evt: Oni.EventContext): void {
         // TODO: Can we get rid of these?
-        this._errorOverlay.onVimEvent(eventName, evt)
         this._liveEvaluationOverlay.onVimEvent(eventName, evt)
-        this._scrollbarOverlay.onVimEvent(eventName, evt)
+
+        UI.Actions.setWindowState(evt.windowNumber, evt.bufferFullPath, evt.column, evt.line, evt.winline, evt.wincol, evt.windowTopLine, evt.windowBottomLine)
+        UI.Actions.setBufferState(evt.bufferFullPath, evt.bufferTotalLines)
 
         this._tasks.onEvent(evt)
 

browser/src/Editor/NeovimSurface.tsx

@@ -11,8 +11,11 @@ import { NeovimInstance } from "./../neovim"
 import { DOMRenderer } from "./../Renderer/DOMRenderer"
 import { NeovimScreen } from "./../Screen"
 
+import { ActiveWindowContainer } from "./../UI/components/ActiveWindow"
+import { ConnectedBufferScrollBar } from "./../UI/components/BufferScrollBar"
 import { Cursor } from "./../UI/components/Cursor"
 import { CursorLine } from "./../UI/components/CursorLine"
+import { ErrorsContainer } from "./../UI/components/Error"
 
 import { NeovimInput } from "./NeovimInput"
 import { NeovimRenderer } from "./NeovimRenderer"
@@ -35,6 +38,12 @@ export class NeovimSurface extends React.PureComponent<INeovimSurfaceProps, void
                 <CursorLine lineType={"line"} />
                 <CursorLine lineType={"column"} />
             </div>
+            <div className="stack layer">
+                <ActiveWindowContainer>
+                    <ErrorsContainer />
+                    <ConnectedBufferScrollBar />
+                </ActiveWindowContainer>
+            </div>
             <NeovimInput neovimInstance={this.props.neovimInstance}
                 screen={this.props.screen} />
         </div>

browser/src/Plugins/Api/Diagnostics.ts

@@ -6,6 +6,8 @@
 
 import { IPluginChannel } from "./Channel"
 
+import * as types from "vscode-languageserver-types"
+
 /**
  * API instance for interacting with Oni (and vim)
  */
@@ -15,8 +17,7 @@ export class Diagnostics implements Oni.Plugin.Diagnostics.Api {
     constructor(private _channel: IPluginChannel) {
     }
 
-    public setErrors(key: string, fileName: string, errors: Oni.Plugin.Diagnostics.Error[], color?: string): void {
-
+    public setErrors(key: string, fileName: string, errors: types.Diagnostic[]): void {
         if (!errors) {
             return
         }
@@ -31,7 +32,6 @@ export class Diagnostics implements Oni.Plugin.Diagnostics.Api {
             key,
             fileName,
             errors,
-            color,
         })
     }
 

browser/src/Plugins/Api/LanguageClient/LanguageClient.ts

@@ -185,15 +185,7 @@ export class LanguageClient {
         this._connection.onNotification(Helpers.ProtocolConstants.TextDocument.PublishDiagnostics, (args) => {
             const diagnostics: types.Diagnostic[] = args.diagnostics
 
-            const oniDiagnostics = diagnostics.map((d) => ({
-                type: null,
-                text: d.message,
-                lineNumber: d.range.start.line + 1,
-                startColumn: d.range.start.character + 1,
-                endColumn: d.range.end.character + 1,
-            }))
-
-            this._oni.diagnostics.setErrors(this._initializationParams.clientName, Helpers.unwrapFileUriPath(args.uri), oniDiagnostics, "red")
+            this._oni.diagnostics.setErrors(this._initializationParams.clientName, Helpers.unwrapFileUriPath(args.uri), diagnostics)
         })
 
         // Register additional notifications here

browser/src/Plugins/PluginManager.ts

@@ -205,7 +205,7 @@ export class PluginManager extends EventEmitter {
                 setTimeout(() => UI.Actions.setDetailedCompletionEntry(pluginResponse.payload.details))
                 break
             case "set-errors":
-                this.emit("set-errors", pluginResponse.payload.key, pluginResponse.payload.fileName, pluginResponse.payload.errors, pluginResponse.payload.color)
+                this.emit("set-errors", pluginResponse.payload.key, pluginResponse.payload.fileName, pluginResponse.payload.errors)
                 break
             case "find-all-references":
                 this.emit("find-all-references", pluginResponse.payload.references)

browser/src/Services/Errors.ts

@@ -1,40 +1,48 @@
 import * as _ from "lodash"
 
 import { INeovimInstance } from "./../neovim"
-import * as Performance from "./../Performance"
 
 import { ITask, ITaskProvider } from "./Tasks"
 
+import * as types from "vscode-languageserver-types"
+
 /**
  * Window that shows terminal output
  */
 
+export const getColorFromSeverity = (severity: types.DiagnosticSeverity): string => {
+    switch (severity) {
+        case types.DiagnosticSeverity.Error:
+            return "red"
+        case types.DiagnosticSeverity.Warning:
+            return "yellow"
+        case types.DiagnosticSeverity.Information:
+        case types.DiagnosticSeverity.Hint:
+        default:
+            return "gray"
+    }
+}
+
 export class Errors implements ITaskProvider {
     private _neovimInstance: INeovimInstance
-    private _errors: { [fileName: string]: Oni.Plugin.Diagnostics.Error[] } = {}
-    private _debouncedSetQuickFix: () => void
+    private _errors: { [fileName: string]: types.Diagnostic[] } = {}
 
     constructor(neovimInstance: INeovimInstance) {
         this._neovimInstance = neovimInstance
-
-        this._debouncedSetQuickFix = _.debounce(() => {
-            Performance.mark("_setQuickFixErrors - begin")
-            this._setQuickFixErrors()
-            Performance.mark("_setQuickFixErrors - end")
-        }, 250)
     }
 
-    public setErrors(fileName: string, errors: Oni.Plugin.Diagnostics.Error[]) {
+    public setErrors(fileName: string, errors: types.Diagnostic[]) {
         this._errors[fileName] = errors
-
-        this._debouncedSetQuickFix()
     }
 
     public getTasks(): Promise<ITask[]> {
         const showErrorTask: ITask = {
             name: "Show Errors",
             detail: "Open quickfix window and show error details",
-            callback: () => this._neovimInstance.command("copen"),
+            callback: () => {
+                this._setQuickFixErrors()
+                this._neovimInstance.command("copen")
+            },
         }
 
         const tasks = [showErrorTask]
@@ -52,9 +60,9 @@ export class Errors implements ITaskProvider {
         const flattenedErrors = _.flatten(arrayOfErrors)
         const errors = flattenedErrors.map((e) => <any>({
             filename: e.filename,
-            col: e.startColumn || 0,
-            lnum: e.lineNumber,
-            text: e.text,
+            col: e.range.start.character || 0,
+            lnum: e.range.start.line,
+            text: e.message,
         }))
 
         this._neovimInstance.quickFix.setqflist(errors, "Errors", " ")

browser/src/UI/ActionCreators.ts

@@ -11,13 +11,71 @@ import * as Events from "./Events"
 import { Rectangle } from "./Types"
 
 import { IScreen } from "./../Screen"
+import { normalizePath } from "./../Utility"
 
 import * as State from "./State"
 
 import * as Config from "./../Config"
 import * as Actions from "./Actions"
 import { events } from "./Events"
-import {ILog} from "./Logs"
+import { ILog } from "./Logs"
+
+import * as types from "vscode-languageserver-types"
+
+export const setBufferState = (file: string, totalLines: number) => ({
+    type: "SET_BUFFER_STATE",
+    payload: {
+        file: normalizePath(file),
+        totalLines,
+    },
+})
+
+export const setWindowState = (windowId: number, file: string, column: number, line: number, winline: number, wincolumn: number, windowTopLine: number, windowBottomLine: number) => ({
+    type: "SET_WINDOW_STATE",
+    payload: {
+        windowId,
+        file: normalizePath(file),
+        column,
+        line,
+        winline,
+        wincolumn,
+        windowTopLine,
+        windowBottomLine,
+    },
+})
+
+export const setWindowLineMapping = (windowId: number, lineMapping: State.WindowLineMap) => ({
+    type: "SET_WINDOW_LINE_MAP",
+    payload: {
+        windowId,
+        lineMapping,
+    },
+})
+
+export const setWindowDimensions = (windowId: number, dimensions: Rectangle) => ({
+    type: "SET_WINDOW_DIMENSIONS",
+    payload: {
+        windowId,
+        dimensions,
+    },
+})
+
+export const setErrors = (file: string, key: string, errors: types.Diagnostic[]) => ({
+    type: "SET_ERRORS",
+    payload: {
+        file: normalizePath(file),
+        key,
+        errors,
+    },
+})
+
+export const clearErrors = (file: string, key: string) => ({
+    type: "CLEAR_ERRORS",
+    payload: {
+        file,
+        key,
+    },
+})
 
 export const showStatusBarItem = (id: string, contents: JSX.Element, alignment?: State.StatusBarAlignment, priority?: number) => ({
     type: "STATUSBAR_SHOW",
@@ -83,11 +141,6 @@ export const setColors = (foregroundColor: string, backgroundColor: string) => (
     dispatch(_setColors(foregroundColor, backgroundColor))
 }
 
-export const setActiveWindowDimensions = (dimensions: Rectangle) => ({
-    type: "SET_ACTIVE_WINDOW_DIMENSIONS",
-    payload: { dimensions },
-})
-
 export const setMode = (mode: string) => ({
     type: "SET_MODE",
     payload: { mode },