fix(5809): test and debug composition divergence with hotkeys. (#5931)

* fix(5809): have commenting hotkeys within composition block, lead to divergence

* chore(5809): in-code documentation of edge case which we are not handling; since we plan to move away from the entire concept of 'diverge' shortly, it should be ok.

* feat(5809): add tests for all conditions triggering divergence, including hotkeys. Include skipped test, with comment, for edge case intentionally not handled.

* chore: remove flaky test which is specific for CI firefox only, and based on a feature we plan to already change in a month.

* chore: convert the login beforeEach to a before

* feat: also make the isLsp heuristic more stringent

* Revert "chore: convert the login beforeEach to a before"

This reverts commit 21e6c8b.

Author: wiedld

cypress/e2e/shared/fluxQueryBuilder.test.ts

@@ -15,6 +15,7 @@ describe('Script Builder', () => {
   const writeData: string[] = []
   for (let i = 0; i < 30; i++) {
     writeData.push(`ndbc,air_temp_degc=70_degrees station_id_${i}=${i}`)
+    writeData.push(`ndbc2,air_temp_degc=70_degrees station_id_${i}=${i}`)
   }
 
   const bucketName = 'defbuck'
@@ -312,7 +313,7 @@ describe('Script Builder', () => {
       })
     })
 
-    describe('default sync and resetting behavior:', () => {
+    describe('sync and resetting behavior:', () => {
       it('sync defaults to on. Can be toggled on/off. And can diverge (be disabled).', () => {
         cy.log('starts as synced')
         cy.getByTestID('flux-sync--toggle').should('have.class', 'active')
@@ -344,7 +345,99 @@ describe('Script Builder', () => {
         )
       })
 
-      it('should clear the editor text, and schema browser, with a new script', () => {
+      describe('conditions for divergence:', () => {
+        it('diverges when typing in composition block', () => {
+          cy.getByTestID('flux-sync--toggle').should('have.class', 'active')
+          cy.getByTestID('flux-editor', {timeout: 30000})
+          selectSchema()
+          confirmSchemaComposition()
+
+          cy.log('does not diverge when outside block')
+          cy.getByTestID('flux-editor').monacoType('// will not diverge')
+          cy.getByTestID('flux-sync--toggle').should(
+            'not.have.class',
+            'disabled'
+          )
+
+          cy.log('does diverge, within block')
+          cy.getByTestID('flux-editor').monacoType(
+            '{enter}{upArrow}{upArrow}{upArrow} // make diverge'
+          )
+          cy.log('toggle is now disabled')
+          cy.getByTestID('flux-sync--toggle').should('have.class', 'disabled')
+        })
+
+        describe('using hotkeys:', () => {
+          const runTest = (hotKeyCombo: string) => {
+            cy.getByTestID('flux-sync--toggle').should('have.class', 'active')
+            cy.getByTestID('flux-editor', {timeout: 30000})
+            selectSchema()
+            confirmSchemaComposition()
+
+            cy.log('does not diverge when outside block')
+            cy.getByTestID('flux-editor').monacoType(`foo ${hotKeyCombo}`)
+            cy.getByTestID('flux-sync--toggle').should(
+              'not.have.class',
+              'disabled'
+            )
+
+            cy.log('does diverge, within block')
+            cy.getByTestID('flux-editor').monacoType(
+              `{upArrow}{upArrow}{upArrow} ${hotKeyCombo}`
+            )
+            cy.log('toggle is now disabled')
+            cy.getByTestID('flux-sync--toggle').should('have.class', 'disabled')
+          }
+
+          it('diverges when commenting line', () => {
+            runTest('{cmd}/')
+          })
+
+          it('diverges when adding lines', () => {
+            runTest('{shift+alt+downArrow}')
+          })
+
+          it('diverges when removing lines', () => {
+            runTest('{cmd+x}')
+          })
+
+          it('diverges when moving lines', () => {
+            runTest('{alt+downArrow}')
+          })
+
+          /// XXX: wiedld (27 Sep 2022) -- we have no way to delineate btwn the LSP applyEdit,
+          /// and the undo action applyEdit.
+          /// Either we disable the undo/redo hotkeys, or accept this edge case bug.
+          it.skip('diverges when using undo hotkeys, to undo composition block change', () => {
+            cy.getByTestID('flux-sync--toggle').should('have.class', 'active')
+            cy.getByTestID('flux-editor', {timeout: 30000})
+            selectSchema()
+            confirmSchemaComposition()
+
+            cy.log('make a change, via schema composition')
+            const newMeasurement = 'ndbc2'
+            cy.getByTestID('measurement-selector--dropdown-button').click()
+            cy.getByTestID(`searchable-dropdown--item ${newMeasurement}`)
+              .should('be.visible')
+              .click()
+            cy.getByTestID('measurement-selector--dropdown-button').should(
+              'contain',
+              newMeasurement
+            )
+            cy.getByTestID('flux-editor').contains(
+              `|> filter(fn: (r) => r._measurement == "${newMeasurement}")`
+            )
+
+            cy.log('use undo hotkey')
+            cy.getByTestID('flux-editor').monacoType('{cmd+z}')
+
+            cy.log('toggle is now disabled')
+            cy.getByTestID('flux-sync--toggle').should('have.class', 'disabled')
+          })
+        })
+      })
+
+      it('should clear the editor text and schema browser, with a new script', () => {
         cy.getByTestID('flux-editor', {timeout: 30000})
 
         cy.log('modify schema browser')

src/languageSupport/languages/flux/lsp/connection.ts

@@ -13,6 +13,7 @@ import {
   SchemaSelection,
 } from 'src/dataExplorer/context/persistance'
 import {CompositionInitParams} from 'src/languageSupport/languages/flux/lsp/utils'
+import {comments} from 'src/languageSupport/languages/flux/monaco.flux.hotkeys'
 
 // LSP methods
 import {
@@ -134,8 +135,19 @@ class LspConnectionManager {
     })
   }
 
+  /// XXX: wiedld (27 Sep 2022) -- This heuristic is wrong.
+  /// Currently, the only way we detect monaco-editor apply changes is with the change object.
+  /// The change object only includes the replacement text, the position, and a little bit of editor-specific metadata.
+  /// The change object does NOT include anything which states whether or not the change is from an LSP-request,
+  /// (via the LSP applyEdit command).
+  /// As a result, we guess that any edit which is replacing the composition block is coming from an LSP request.
+  /// However, this heuristic fails for certain user-triggered events.
+  /// Such as a hotkey "undo" [cmd+z] of the last LSP applyEdit,
+  /// which generates a change object that looks exactly like an older applyEdit change.
   _editorChangeIsFromLsp(change) {
-    return change.text?.includes('|> yield(name: "_editor_composition")')
+    return /^(from)(.|\n)*(\|> yield\(name: "_editor_composition"\)\n)$/.test(
+      change.text
+    )
   }
 
   _editorChangeIsWithinComposition(change) {
@@ -179,6 +191,22 @@ class LspConnectionManager {
         })
       }
     })
+
+    comments(this._editor, selection => {
+      const compositionBlock = this._getCompositionBlockLines()
+      if (!compositionBlock) {
+        return
+      }
+      const {startLine, endLine} = compositionBlock
+      if (
+        selection.startLineNumber >= startLine &&
+        selection.endLineNumber <= endLine
+      ) {
+        this._callbackSetSession({
+          composition: {synced: false, diverged: true},
+        })
+      }
+    })
   }
 
   _compositionSyncStyle(startLine: number, endLine: number, synced: boolean) {

src/languageSupport/languages/flux/monaco.flux.hotkeys.ts

@@ -1,4 +1,4 @@
-import {EditorType} from 'src/types'
+import {EditorType, MonacoSelectionRange} from 'src/types'
 
 export const toggleCommenting = (s: string, isTogglingOn: boolean) => {
   if (isTogglingOn) {
@@ -9,7 +9,10 @@ export const toggleCommenting = (s: string, isTogglingOn: boolean) => {
 
 export const isCommented = (s: string) => !!s.match(/^\s*(\/\/(.*)$)/g)
 
-export function comments(editor: EditorType) {
+export function comments(
+  editor: EditorType,
+  cb: (x: MonacoSelectionRange) => void = _ => {}
+) {
   editor.addAction({
     // An unique identifier of the contributed action.
     id: 'toggle-comment',
@@ -49,6 +52,8 @@ export function comments(editor: EditorType) {
           : positionColumn - 3,
       })
 
+      cb(selection)
+
       return null
     },
   })

src/types/monaco.ts

@@ -6,3 +6,4 @@ export type EditorType = allMonaco.editor.IStandaloneCodeEditor
 export type CursorEvent = allMonaco.editor.ICursorPositionChangedEvent
 export type KeyboardEvent = allMonaco.IKeyboardEvent
 export type MonacoRange = allMonaco.Range
+export type MonacoSelectionRange = allMonaco.Selection