Merge branch 'himkp-monaco-model-configuration' into 'master'

Prepare Editor Model to accept options

See merge request gitlab-org/gitlab!32857

app/assets/javascripts/ide/components/repo_editor.vue

@@ -14,7 +14,6 @@ import Editor from '../lib/editor';
 import FileTemplatesBar from './file_templates/bar.vue';
 import { __ } from '~/locale';
 import { extractMarkdownImagesFromEntries } from '../stores/utils';
-import { addFinalNewline } from '../utils';
 
 export default {
   components: {
@@ -32,7 +31,6 @@ export default {
     return {
       content: '',
       images: {},
-      addFinalNewline: true,
     };
   },
   computed: {
@@ -253,10 +251,8 @@ export default {
 
         const monacoModel = model.getModel();
         const content = monacoModel.getValue();
-        this.changeFileContent({
-          path: file.path,
-          content: this.addFinalNewline ? addFinalNewline(content, monacoModel.getEOL()) : content,
-        });
+        this.changeFileContent({ path: file.path, content });
+        this.setFileEOL({ eol: this.model.eol });
       });
 
       // Handle Cursor Position

app/assets/javascripts/ide/lib/common/model.js

 import { editor as monacoEditor, Uri } from 'monaco-editor';
 import Disposable from './disposable';
 import eventHub from '../../eventhub';
+import { trimTrailingWhitespace, insertFinalNewline } from '../../utils';
+import { defaultModelOptions } from '../editor_options';
 
 export default class Model {
   constructor(file, head = null) {
@@ -8,6 +10,7 @@ export default class Model {
     this.file = file;
     this.head = head;
     this.content = file.content !== '' || file.deleted ? file.content : file.raw;
+    this.options = { ...defaultModelOptions };
 
     this.disposable.add(
       (this.originalModel = monacoEditor.createModel(
@@ -94,8 +97,32 @@ export default class Model {
     this.getModel().setValue(content);
   }
 
+  updateOptions(obj = {}) {
+    Object.assign(this.options, obj);
+    this.model.updateOptions(obj);
+    this.applyCustomOptions();
+  }
+
+  applyCustomOptions() {
+    this.updateNewContent(
+      Object.entries(this.options).reduce((content, [key, value]) => {
+        switch (key) {
+          case 'endOfLine':
+            this.model.pushEOL(value);
+            return this.model.getValue();
+          case 'insertFinalNewline':
+            return value ? insertFinalNewline(content) : content;
+          case 'trimTrailingWhitespace':
+            return value ? trimTrailingWhitespace(content) : content;
+          default:
+            return content;
+        }
+      }, this.model.getValue()),
+    );
+  }
+
   dispose() {
-    this.disposable.dispose();
+    if (!this.model.isDisposed()) this.applyCustomOptions();
 
     this.events.forEach(cb => {
       if (typeof cb === 'function') cb();
@@ -106,5 +133,7 @@ export default class Model {
     eventHub.$off(`editor.update.model.dispose.${this.file.key}`, this.dispose);
     eventHub.$off(`editor.update.model.content.${this.file.key}`, this.updateContent);
     eventHub.$off(`editor.update.model.new.content.${this.file.key}`, this.updateNewContent);
+
+    this.disposable.dispose();
   }
 }

app/assets/javascripts/ide/lib/diff/controller.js

@@ -50,10 +50,15 @@ export default class DirtyDiffController {
   }
 
   computeDiff(model) {
+    const originalModel = model.getOriginalModel();
+    const newModel = model.getModel();
+
+    if (originalModel.isDisposed() || newModel.isDisposed()) return;
+
     this.dirtyDiffWorker.postMessage({
       path: model.path,
-      originalContent: model.getOriginalModel().getValue(),
-      newContent: model.getModel().getValue(),
+      originalContent: originalModel.getValue(),
+      newContent: newModel.getValue(),
     });
   }
 

app/assets/javascripts/ide/lib/diff/diff.js

 import { diffLines } from 'diff';
+import { defaultDiffOptions } from '../editor_options';
 
+// See: https://gitlab.com/gitlab-org/frontend/rfcs/-/issues/20
 // eslint-disable-next-line import/prefer-default-export
 export const computeDiff = (originalContent, newContent) => {
-  const changes = diffLines(originalContent, newContent);
+  // prevent EOL changes from highlighting the entire file
+  const changes = diffLines(
+    originalContent.replace(/\r\n/g, '\n'),
+    newContent.replace(/\r\n/g, '\n'),
+    defaultDiffOptions,
+  );
 
   let lineNumber = 1;
   return changes.reduce((acc, change) => {

app/assets/javascripts/ide/lib/editor.js

@@ -5,7 +5,7 @@ import DecorationsController from './decorations/controller';
 import DirtyDiffController from './diff/controller';
 import Disposable from './common/disposable';
 import ModelManager from './common/model_manager';
-import editorOptions, { defaultEditorOptions } from './editor_options';
+import { editorOptions, defaultEditorOptions, defaultDiffEditorOptions } from './editor_options';
 import { themes } from './themes';
 import languages from './languages';
 import keymap from './keymap.json';
@@ -73,8 +73,7 @@ export default class Editor {
       this.disposable.add(
         (this.instance = monacoEditor.createDiffEditor(domElement, {
           ...this.options,
-          quickSuggestions: false,
-          occurrencesHighlight: false,
+          ...defaultDiffEditorOptions,
           renderSideBySide: Editor.renderSideBySide(domElement),
           readOnly,
           renderLineHighlight: readOnly ? 'all' : 'none',

app/assets/javascripts/ide/lib/editor_options.js

@@ -9,7 +9,23 @@ export const defaultEditorOptions = {
   wordWrap: 'on',
 };
 
-export default [
+export const defaultDiffOptions = {
+  ignoreWhitespace: false,
+};
+
+export const defaultDiffEditorOptions = {
+  quickSuggestions: false,
+  occurrencesHighlight: false,
+  ignoreTrimWhitespace: false,
+};
+
+export const defaultModelOptions = {
+  endOfLine: 0,
+  insertFinalNewline: true,
+  trimTrailingWhitespace: false,
+};
+
+export const editorOptions = [
   {
     readOnly: model => Boolean(model.file.file_lock),
     quickSuggestions: model => !(model.language === 'markdown'),

app/assets/javascripts/ide/utils.js

@@ -77,7 +77,11 @@ export function registerLanguages(def, ...defs) {
 
 export const otherSide = side => (side === SIDE_RIGHT ? SIDE_LEFT : SIDE_RIGHT);
 
-export function addFinalNewline(content, eol = '\n') {
+export function trimTrailingWhitespace(content) {
+  return content.replace(/[^\S\r\n]+$/gm, '');
+}
+
+export function insertFinalNewline(content, eol = '\n') {
   return content.slice(-eol.length) !== eol ? `${content}${eol}` : content;
 }
 

spec/frontend/ide/components/repo_editor_spec.js

@@ -283,25 +283,13 @@ describe('RepoEditor', () => {
       expect(vm.model.events.size).toBe(2);
     });
 
-    it.each`
-      insertFinalNewline | input              | eol       | output
-      ${true}            | ${'testing 123\n'} | ${'\n'}   | ${'testing 123\n'}
-      ${true}            | ${'testing 123'}   | ${'\n'}   | ${'testing 123\n'}
-      ${false}           | ${'testing 123'}   | ${'\n'}   | ${'testing 123'}
-      ${true}            | ${'testing 123'}   | ${'\r\n'} | ${'testing 123\r\n'}
-      ${false}           | ${'testing 123'}   | ${'\r\n'} | ${'testing 123'}
-    `(
-      'updates state with "$output" if `this.insertFinalNewline` is $insertFinalNewline',
-      ({ insertFinalNewline, input, eol, output }) => {
-        jest.spyOn(vm.model.getModel(), 'getEOL').mockReturnValue(eol);
-
-        vm.addFinalNewline = insertFinalNewline;
-
-        vm.model.setValue(input);
-
-        expect(vm.file.content).toBe(output);
-      },
-    );
+    it('updates state with the value of the model', () => {
+      vm.model.setValue('testing 1234');
+
+      vm.setupEditor();
+
+      expect(vm.file.content).toBe('testing 1234');
+    });
 
     it('sets head model as staged file', () => {
       jest.spyOn(vm.editor, 'createModel');

spec/frontend/ide/lib/common/model_spec.js

@@ -133,5 +133,77 @@ describe('Multi-file editor library model', () => {
 
       expect(disposeSpy).toHaveBeenCalled();
     });
+
+    it('applies custom options and triggers onChange callback', () => {
+      const changeSpy = jest.fn();
+      jest.spyOn(model, 'applyCustomOptions');
+
+      model.onChange(changeSpy);
+
+      model.dispose();
+
+      expect(model.applyCustomOptions).toHaveBeenCalled();
+      expect(changeSpy).toHaveBeenCalled();
+    });
+  });
+
+  describe('updateOptions', () => {
+    it('sets the options on the options object', () => {
+      model.updateOptions({ insertSpaces: true, someOption: 'some value' });
+
+      expect(model.options).toEqual({
+        endOfLine: 0,
+        insertFinalNewline: true,
+        insertSpaces: true,
+        someOption: 'some value',
+        trimTrailingWhitespace: false,
+      });
+    });
+
+    it.each`
+      option            | value
+      ${'insertSpaces'} | ${true}
+      ${'insertSpaces'} | ${false}
+      ${'indentSize'}   | ${4}
+      ${'tabSize'}      | ${3}
+    `("correctly sets option: $option=$value to Monaco's TextModel", ({ option, value }) => {
+      model.updateOptions({ [option]: value });
+
+      expect(model.getModel().getOptions()).toMatchObject({ [option]: value });
+    });
+
+    it('applies custom options immediately', () => {
+      jest.spyOn(model, 'applyCustomOptions');
+
+      model.updateOptions({ trimTrailingWhitespace: true, someOption: 'some value' });
+
+      expect(model.applyCustomOptions).toHaveBeenCalled();
+    });
+  });
+
+  describe('applyCustomOptions', () => {
+    it.each`
+      option                      | value    | contentBefore                   | contentAfter
+      ${'endOfLine'}              | ${0}     | ${'hello\nworld\n'}             | ${'hello\nworld\n'}
+      ${'endOfLine'}              | ${0}     | ${'hello\r\nworld\r\n'}         | ${'hello\nworld\n'}
+      ${'endOfLine'}              | ${1}     | ${'hello\nworld\n'}             | ${'hello\r\nworld\r\n'}
+      ${'endOfLine'}              | ${1}     | ${'hello\r\nworld\r\n'}         | ${'hello\r\nworld\r\n'}
+      ${'insertFinalNewline'}     | ${true}  | ${'hello\nworld'}               | ${'hello\nworld\n'}
+      ${'insertFinalNewline'}     | ${true}  | ${'hello\nworld\n'}             | ${'hello\nworld\n'}
+      ${'insertFinalNewline'}     | ${false} | ${'hello\nworld'}               | ${'hello\nworld'}
+      ${'trimTrailingWhitespace'} | ${true}  | ${'hello  \t\nworld  \t\n'}     | ${'hello\nworld\n'}
+      ${'trimTrailingWhitespace'} | ${true}  | ${'hello  \t\r\nworld  \t\r\n'} | ${'hello\nworld\n'}
+      ${'trimTrailingWhitespace'} | ${false} | ${'hello  \t\r\nworld  \t\r\n'} | ${'hello  \t\nworld  \t\n'}
+    `(
+      'correctly applies custom option $option=$value to content',
+      ({ option, value, contentBefore, contentAfter }) => {
+        model.options[option] = value;
+
+        model.updateNewContent(contentBefore);
+        model.applyCustomOptions();
+
+        expect(model.getModel().getValue()).toEqual(contentAfter);
+      },
+    );
   });
 });

spec/frontend/ide/lib/diff/diff_spec.js

@@ -73,5 +73,13 @@ describe('Multi-file editor library diff calculator', () => {
 
       expect(diff.endLineNumber).toBe(1);
     });
+
+    it('disregards changes for EOL type changes', () => {
+      const text1 = 'line1\nline2\nline3\n';
+      const text2 = 'line1\r\nline2\r\nline3\r\n';
+
+      expect(computeDiff(text1, text2)).toEqual([]);
+      expect(computeDiff(text2, text1)).toEqual([]);
+    });
   });
 });

spec/frontend/ide/lib/editor_options_spec.js

-import editorOptions from '~/ide/lib/editor_options';
-
-describe('Multi-file editor library editor options', () => {
-  it('returns an array', () => {
-    expect(editorOptions).toEqual(expect.any(Array));
-  });
-
-  it('contains readOnly option', () => {
-    expect(editorOptions[0].readOnly).toBeDefined();
-  });
-});

spec/frontend/ide/lib/editor_spec.js

@@ -72,6 +72,7 @@ describe('Multi-file editor library', () => {
 
       expect(monacoEditor.createDiffEditor).toHaveBeenCalledWith(holder, {
         ...defaultEditorOptions,
+        ignoreTrimWhitespace: false,
         quickSuggestions: false,
         occurrencesHighlight: false,
         renderSideBySide: false,

spec/frontend/ide/utils_spec.js

@@ -2,7 +2,8 @@ import {
   isTextFile,
   registerLanguages,
   trimPathComponents,
-  addFinalNewline,
+  insertFinalNewline,
+  trimTrailingWhitespace,
   getPathParents,
 } from '~/ide/utils';
 import { languages } from 'monaco-editor';
@@ -155,6 +156,20 @@ describe('WebIDE utils', () => {
     });
   });
 
+  describe('trimTrailingWhitespace', () => {
+    it.each`
+      input                                                            | output
+      ${'text     \n   more text   \n'}                                | ${'text\n   more text\n'}
+      ${'text     \n   more text   \n\n   \n'}                         | ${'text\n   more text\n\n\n'}
+      ${'text  \t\t   \n   more text   \n\t\ttext\n   \n\t\t'}         | ${'text\n   more text\n\t\ttext\n\n'}
+      ${'text     \r\n   more text   \r\n'}                            | ${'text\r\n   more text\r\n'}
+      ${'text     \r\n   more text   \r\n\r\n   \r\n'}                 | ${'text\r\n   more text\r\n\r\n\r\n'}
+      ${'text  \t\t   \r\n   more text   \r\n\t\ttext\r\n   \r\n\t\t'} | ${'text\r\n   more text\r\n\t\ttext\r\n\r\n'}
+    `("trims trailing whitespace in each line of file's contents: $input", ({ input, output }) => {
+      expect(trimTrailingWhitespace(input)).toBe(output);
+    });
+  });
+
   describe('addFinalNewline', () => {
     it.each`
       input              | output
@@ -163,7 +178,7 @@ describe('WebIDE utils', () => {
       ${'some text\n\n'} | ${'some text\n\n'}
       ${'some\n text'}   | ${'some\n text\n'}
     `('adds a newline if it doesnt already exist for input: $input', ({ input, output }) => {
-      expect(addFinalNewline(input)).toEqual(output);
+      expect(insertFinalNewline(input)).toBe(output);
     });
 
     it.each`
@@ -174,7 +189,7 @@ describe('WebIDE utils', () => {
       ${'some text\r\n\r\n'} | ${'some text\r\n\r\n'}
       ${'some\r\n text'}     | ${'some\r\n text\r\n'}
     `('works with CRLF newline style; input: $input', ({ input, output }) => {
-      expect(addFinalNewline(input, '\r\n')).toEqual(output);
+      expect(insertFinalNewline(input, '\r\n')).toBe(output);
     });
   });