From e5ad2a8724e59c7ca633226fa4035343fa83fe67 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Tue, 3 Jul 2018 12:14:51 +0100 Subject: [PATCH 1/5] Abstract Ace-specific code to adapter --- .../ide/editor/directives/aceEditor.coffee | 22 +++++- .../CursorPositionAdapter.coffee | 35 ++++++++ .../CursorPositionManager.coffee | 79 ++++++++----------- 3 files changed, 87 insertions(+), 49 deletions(-) create mode 100644 services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index 617b41b845..ef6db538c4 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -10,13 +10,14 @@ define [ "ide/editor/directives/aceEditor/spell-check/SpellCheckAdapter" "ide/editor/directives/aceEditor/highlights/HighlightsManager" "ide/editor/directives/aceEditor/cursor-position/CursorPositionManager" + "ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter" "ide/editor/directives/aceEditor/track-changes/TrackChangesManager" "ide/editor/directives/aceEditor/metadata/MetadataManager" "ide/metadata/services/metadata" "ide/graphics/services/graphics" "ide/preamble/services/preamble" "ide/files/services/files" -], (App, Ace, SearchBox, Vim, ModeList, UndoManager, AutoCompleteManager, SpellCheckManager, SpellCheckAdapter, HighlightsManager, CursorPositionManager, TrackChangesManager, MetadataManager) -> +], (App, Ace, SearchBox, Vim, ModeList, UndoManager, AutoCompleteManager, SpellCheckManager, SpellCheckAdapter, HighlightsManager, CursorPositionManager, CursorPositionAdapter, TrackChangesManager, MetadataManager) -> EditSession = ace.require('ace/edit_session').EditSession ModeList = ace.require('ace/ext/modelist') Vim = ace.require('ace/keyboard/vim').Vim @@ -108,7 +109,7 @@ define [ undoManager = new UndoManager(scope, editor, element) highlightsManager = new HighlightsManager(scope, editor, element) - cursorPositionManager = new CursorPositionManager(scope, editor, element, localStorage) + cursorPositionManager = new CursorPositionManager(scope, new CursorPositionAdapter(editor), localStorage) trackChangesManager = new TrackChangesManager(scope, editor, element) metadataManager = new MetadataManager(scope, editor, element, metadata) autoCompleteManager = new AutoCompleteManager(scope, editor, element, metadataManager, graphics, preamble, files) @@ -380,6 +381,21 @@ define [ editor.off 'changeSession', onSessionChangeForSpellCheck editor.off 'nativecontextmenu', spellCheckManager.onContextMenu + onSessionChangeForCursorPosition = (e) -> + cursorPositionManager.onSessionChange(e) + e.oldSession?.selection.off 'changeCursor', cursorPositionManager.onCursorChange + e.session.selection.on 'changeCursor', cursorPositionManager.onCursorChange + + initCursorPosition = () -> + cursorPositionManager.init() + editor.on 'changeSession', onSessionChangeForCursorPosition + onSessionChangeForCursorPosition({ session: editor.getSession() }) # Force initial setup + $(window).on "unload", () -> + cursorPositionManager.onUnload(editor.getSession()) + + tearDownCursorPosition = () -> + editor.off 'changeSession', onSessionChangeForCursorPosition + attachToAce = (sharejs_doc) -> lines = sharejs_doc.getSnapshot().split("\n") session = editor.getSession() @@ -426,6 +442,7 @@ define [ # now ready to edit document editor.setReadOnly(scope.readOnly) # respect the readOnly setting, normally false initSpellCheck() + initCursorPosition() resetScrollMargins() @@ -488,6 +505,7 @@ define [ scope.$on '$destroy', () -> if scope.sharejsDoc? tearDownSpellCheck() + tearDownCursorPosition() detachFromAce(scope.sharejsDoc) session = editor.getSession() session?.destroy() diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee new file mode 100644 index 0000000000..6c8c051f8a --- /dev/null +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee @@ -0,0 +1,35 @@ +define [ + "ide/editor/AceShareJsCodec" +], (AceShareJsCodec) -> + class CursorPositionAdapter + constructor: (@editor) -> + + getCursor: () -> + @editor.getCursorPosition() + + getCursorForSession: (session) -> + session.selection.getCursor() + + getScrollTopForSession: (session) -> + session.getScrollTop() + + setCursor: (pos) -> + pos = pos.cursorPosition or { row: 0, column: 0 } + @editor.moveCursorToPosition(pos) + + setScrollTop: (pos) -> + pos = pos.scrollTop or 0 + @editor.getSession().setScrollTop(pos) + + clearSelection: () -> + @editor.selection.clearSelection() + + gotoLine: (line, column) -> + @editor.gotoLine(line, column) + @editor.scrollToLine(line, true, true) # centre and animate + @editor.focus() + + gotoOffset: (offset) -> + lines = @editor.getSession().getDocument().getAllLines() + position = AceShareJsCodec.shareJsOffsetToAcePosition(offset, lines) + @gotoLine(position.row + 1, position.column) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee index 119aa471e1..2b6ab3eeaf 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee @@ -1,75 +1,60 @@ -define [ - "ide/editor/AceShareJsCodec" -], (AceShareJsCodec) -> +define [], () -> class CursorPositionManager - constructor: (@$scope, @editor, @element, @localStorage) -> - - onChangeCursor = (e) => - @emitCursorUpdateEvent(e) - - @editor.on "changeSession", (e) => - if e.oldSession? - @storeCursorPosition(e.oldSession) - @storeScrollTopPosition(e.oldSession) - - @doc_id = @$scope.sharejsDoc?.doc_id - - e.oldSession?.selection.off 'changeCursor', onChangeCursor - e.session.selection.on 'changeCursor', onChangeCursor - - setTimeout () => - @gotoStoredPosition() - , 0 - - $(window).on "unload", () => - @storeCursorPosition(@editor.getSession()) - @storeScrollTopPosition(@editor.getSession()) - + constructor: (@$scope, @adapter, @localStorage) -> @$scope.$on "#{@$scope.name}:gotoLine", (e, line, column) => if line? setTimeout () => - @gotoLine(line, column) + @adapter.gotoLine(line, column) , 10 # Hack: Must happen after @gotoStoredPosition @$scope.$on "#{@$scope.name}:gotoOffset", (e, offset) => if offset? setTimeout () => - @gotoOffset(offset) + @adapter.gotoOffset(offset) , 10 # Hack: Must happen after @gotoStoredPosition @$scope.$on "#{@$scope.name}:clearSelection", (e) => - @editor.selection.clearSelection() + @adapter.clearSelection() + + init: () -> + @emitCursorUpdateEvent() + + onSessionChange: (e) => + if e.oldSession? + @storeCursorPosition(e.oldSession) + @storeScrollTopPosition(e.oldSession) + + @doc_id = @$scope.sharejsDoc?.doc_id + + setTimeout () => + @gotoStoredPosition() + , 0 + + onUnload: (session) => + @storeCursorPosition(session) + @storeScrollTopPosition(session) + + onCursorChange: () => + @emitCursorUpdateEvent() storeScrollTopPosition: (session) -> if @doc_id? docPosition = @localStorage("doc.position.#{@doc_id}") || {} - docPosition.scrollTop = session.getScrollTop() + docPosition.scrollTop = @adapter.getScrollTopForSession(session) @localStorage("doc.position.#{@doc_id}", docPosition) storeCursorPosition: (session) -> if @doc_id? docPosition = @localStorage("doc.position.#{@doc_id}") || {} - docPosition.cursorPosition = session.selection.getCursor() + docPosition.cursorPosition = @adapter.getCursorForSession(session) @localStorage("doc.position.#{@doc_id}", docPosition) - + emitCursorUpdateEvent: () -> - cursor = @editor.getCursorPosition() + cursor = @adapter.getCursor() @$scope.$emit "cursor:#{@$scope.name}:update", cursor gotoStoredPosition: () -> return if !@doc_id? pos = @localStorage("doc.position.#{@doc_id}") || {} - @ignoreCursorPositionChanges = true - @editor.moveCursorToPosition(pos.cursorPosition or {row: 0, column: 0}) - @editor.getSession().setScrollTop(pos.scrollTop or 0) - delete @ignoreCursorPositionChanges - - gotoLine: (line, column) -> - @editor.gotoLine(line, column) - @editor.scrollToLine(line,true,true) # centre and animate - @editor.focus() - - gotoOffset: (offset) -> - lines = @editor.getSession().getDocument().getAllLines() - position = AceShareJsCodec.shareJsOffsetToAcePosition(offset, lines) - @gotoLine(position.row + 1, position.column) \ No newline at end of file + @adapter.setCursor(pos) + @adapter.setScrollTop(pos) From 6c7e942470896e8266050231e37cb7f6ec78e161 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 5 Jul 2018 14:25:58 +0100 Subject: [PATCH 2/5] Unbind unload listener when destroying editor --- .../public/coffee/ide/editor/directives/aceEditor.coffee | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index ef6db538c4..b92e7bfb0c 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -386,15 +386,18 @@ define [ e.oldSession?.selection.off 'changeCursor', cursorPositionManager.onCursorChange e.session.selection.on 'changeCursor', cursorPositionManager.onCursorChange + onUnloadForCursorPosition = () -> + cursorPositionManager.onUnload(editor.getSession()) + initCursorPosition = () -> cursorPositionManager.init() editor.on 'changeSession', onSessionChangeForCursorPosition onSessionChangeForCursorPosition({ session: editor.getSession() }) # Force initial setup - $(window).on "unload", () -> - cursorPositionManager.onUnload(editor.getSession()) + $(window).on "unload", onUnloadForCursorPosition tearDownCursorPosition = () -> editor.off 'changeSession', onSessionChangeForCursorPosition + $(window).off "unload", onUnloadForCursorPosition attachToAce = (sharejs_doc) -> lines = sharejs_doc.getSnapshot().split("\n") From ba9fa9a0beddd5c18d18c9d3df8115b4cb059fc8 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 5 Jul 2018 17:24:45 +0100 Subject: [PATCH 3/5] Store first visible line instead of scrollTop scrollTop is affected by changing viewport size and switching between Ace and CM --- .../cursor-position/CursorPositionAdapter.coffee | 13 +++++-------- .../cursor-position/CursorPositionManager.coffee | 10 +++++----- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee index 6c8c051f8a..d58f012b57 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionAdapter.coffee @@ -7,19 +7,16 @@ define [ getCursor: () -> @editor.getCursorPosition() - getCursorForSession: (session) -> - session.selection.getCursor() - - getScrollTopForSession: (session) -> - session.getScrollTop() + getEditorScrollPosition: () -> + @editor.getFirstVisibleRow() setCursor: (pos) -> pos = pos.cursorPosition or { row: 0, column: 0 } @editor.moveCursorToPosition(pos) - setScrollTop: (pos) -> - pos = pos.scrollTop or 0 - @editor.getSession().setScrollTop(pos) + setEditorScrollPosition: (pos) -> + pos = pos.firstVisibleLine or 0 + @editor.scrollToLine(pos) clearSelection: () -> @editor.selection.clearSelection() diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee index 2b6ab3eeaf..b7ba730507 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee @@ -22,7 +22,7 @@ define [], () -> onSessionChange: (e) => if e.oldSession? @storeCursorPosition(e.oldSession) - @storeScrollTopPosition(e.oldSession) + @storeFirstVisibleLine() @doc_id = @$scope.sharejsDoc?.doc_id @@ -32,15 +32,15 @@ define [], () -> onUnload: (session) => @storeCursorPosition(session) - @storeScrollTopPosition(session) + @storeFirstVisibleLine() onCursorChange: () => @emitCursorUpdateEvent() - storeScrollTopPosition: (session) -> + storeFirstVisibleLine: () -> if @doc_id? docPosition = @localStorage("doc.position.#{@doc_id}") || {} - docPosition.scrollTop = @adapter.getScrollTopForSession(session) + docPosition.firstVisibleLine = @adapter.getEditorScrollPosition() @localStorage("doc.position.#{@doc_id}", docPosition) storeCursorPosition: (session) -> @@ -57,4 +57,4 @@ define [], () -> return if !@doc_id? pos = @localStorage("doc.position.#{@doc_id}") || {} @adapter.setCursor(pos) - @adapter.setScrollTop(pos) + @adapter.setEditorScrollPosition(pos) From da77c067744b01f6a87286717fda054b341afe58 Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Thu, 5 Jul 2018 17:37:53 +0100 Subject: [PATCH 4/5] Refactor saving cursor position to not use Ace event This is will help with triggering CM correctly --- .../coffee/ide/editor/directives/aceEditor.coffee | 3 ++- .../cursor-position/CursorPositionManager.coffee | 14 +++++++------- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index b92e7bfb0c..fadc5a4a03 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -308,6 +308,8 @@ define [ editor.renderer.updateFontSize() scope.$watch "sharejsDoc", (sharejs_doc, old_sharejs_doc) -> + cursorPositionManager.onBeforeSessionChange(!!old_sharejs_doc) + if old_sharejs_doc? detachFromAce(old_sharejs_doc) @@ -382,7 +384,6 @@ define [ editor.off 'nativecontextmenu', spellCheckManager.onContextMenu onSessionChangeForCursorPosition = (e) -> - cursorPositionManager.onSessionChange(e) e.oldSession?.selection.off 'changeCursor', cursorPositionManager.onCursorChange e.session.selection.on 'changeCursor', cursorPositionManager.onCursorChange diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee index b7ba730507..57acb81546 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee @@ -19,9 +19,9 @@ define [], () -> init: () -> @emitCursorUpdateEvent() - onSessionChange: (e) => - if e.oldSession? - @storeCursorPosition(e.oldSession) + onBeforeSessionChange: (hasPrevSession = false) => + if hasPrevSession + @storeCursorPosition() @storeFirstVisibleLine() @doc_id = @$scope.sharejsDoc?.doc_id @@ -30,8 +30,8 @@ define [], () -> @gotoStoredPosition() , 0 - onUnload: (session) => - @storeCursorPosition(session) + onUnload: () => + @storeCursorPosition() @storeFirstVisibleLine() onCursorChange: () => @@ -43,10 +43,10 @@ define [], () -> docPosition.firstVisibleLine = @adapter.getEditorScrollPosition() @localStorage("doc.position.#{@doc_id}", docPosition) - storeCursorPosition: (session) -> + storeCursorPosition: () -> if @doc_id? docPosition = @localStorage("doc.position.#{@doc_id}") || {} - docPosition.cursorPosition = @adapter.getCursorForSession(session) + docPosition.cursorPosition = @adapter.getCursor() @localStorage("doc.position.#{@doc_id}", docPosition) emitCursorUpdateEvent: () -> From 5806101bd0223f77093e43928ef5e72591eb03ce Mon Sep 17 00:00:00 2001 From: Alasdair Smith Date: Fri, 6 Jul 2018 15:37:51 +0100 Subject: [PATCH 5/5] Trigger events instead of calling cursor manager This improves readability and prevents race conditions in compat between Ace/CM --- .../ide/editor/directives/aceEditor.coffee | 19 +++++++++++++------ .../CursorPositionManager.coffee | 18 +++++++++--------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee index fadc5a4a03..34da37b6e4 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor.coffee @@ -16,7 +16,7 @@ define [ "ide/metadata/services/metadata" "ide/graphics/services/graphics" "ide/preamble/services/preamble" - "ide/files/services/files" + "ide/files/services/files" ], (App, Ace, SearchBox, Vim, ModeList, UndoManager, AutoCompleteManager, SpellCheckManager, SpellCheckAdapter, HighlightsManager, CursorPositionManager, CursorPositionAdapter, TrackChangesManager, MetadataManager) -> EditSession = ace.require('ace/edit_session').EditSession ModeList = ace.require('ace/ext/modelist') @@ -308,13 +308,13 @@ define [ editor.renderer.updateFontSize() scope.$watch "sharejsDoc", (sharejs_doc, old_sharejs_doc) -> - cursorPositionManager.onBeforeSessionChange(!!old_sharejs_doc) - if old_sharejs_doc? + scope.$broadcast('beforeChangeDocument') detachFromAce(old_sharejs_doc) - if sharejs_doc? attachToAce(sharejs_doc) + if sharejs_doc? and old_sharejs_doc? + scope.$broadcast('afterChangeDocument') scope.$watch "text", (text) -> if text? @@ -391,7 +391,6 @@ define [ cursorPositionManager.onUnload(editor.getSession()) initCursorPosition = () -> - cursorPositionManager.init() editor.on 'changeSession', onSessionChangeForCursorPosition onSessionChangeForCursorPosition({ session: editor.getSession() }) # Force initial setup $(window).on "unload", onUnloadForCursorPosition @@ -400,6 +399,14 @@ define [ editor.off 'changeSession', onSessionChangeForCursorPosition $(window).off "unload", onUnloadForCursorPosition + initCursorPosition() + + # Trigger the event once *only* - this is called after Ace is connected + # to the ShareJs instance but this event should only be triggered the + # first time the editor is opened. Not every time the docs opened + triggerEditorInitEvent = _.once () -> + scope.$broadcast('editorInit') + attachToAce = (sharejs_doc) -> lines = sharejs_doc.getSnapshot().split("\n") session = editor.getSession() @@ -445,8 +452,8 @@ define [ editor.initing = false # now ready to edit document editor.setReadOnly(scope.readOnly) # respect the readOnly setting, normally false + triggerEditorInitEvent() initSpellCheck() - initCursorPosition() resetScrollMargins() diff --git a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee index 57acb81546..2cf025f70e 100644 --- a/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee +++ b/services/web/public/coffee/ide/editor/directives/aceEditor/cursor-position/CursorPositionManager.coffee @@ -1,6 +1,14 @@ define [], () -> class CursorPositionManager constructor: (@$scope, @adapter, @localStorage) -> + @$scope.$on 'editorInit', @jumpToPositionInNewDoc + + @$scope.$on 'beforeChangeDocument', () => + @storeCursorPosition() + @storeFirstVisibleLine() + + @$scope.$on 'afterChangeDocument', @jumpToPositionInNewDoc + @$scope.$on "#{@$scope.name}:gotoLine", (e, line, column) => if line? setTimeout () => @@ -16,16 +24,8 @@ define [], () -> @$scope.$on "#{@$scope.name}:clearSelection", (e) => @adapter.clearSelection() - init: () -> - @emitCursorUpdateEvent() - - onBeforeSessionChange: (hasPrevSession = false) => - if hasPrevSession - @storeCursorPosition() - @storeFirstVisibleLine() - + jumpToPositionInNewDoc: () => @doc_id = @$scope.sharejsDoc?.doc_id - setTimeout () => @gotoStoredPosition() , 0