Content goes inconsistent between MacOS and Windows

[oliverlin]

Version info
Firebase: 3.3.0

Firepad: 1.5.0

Monaco: 0.14.3

MacOS: 10.13

Windows: 10

Steps to reproduce:

1. Host example/monaco.html in localhost server and use ngrok or something similar to host this file for external access.
2. Open this url on on MacOS chrome browser and copy & paste to Windows chrome browser try to have both OS browsers connecting to the same firepad.
3. Copy and paste a simple javascript function to the Windows firpad.

function appendOffset (num) {
  const offset = 1
  return num + offset
}

4. Type any character in the MacOS firepad
5. It would throw an error in the console and the content starts appearing inconsistent between MacOS and Windows firepad

After looked into the content it committed to firebase, I think it is because the linebreak between different OS Windows(\r\n) and MacOS(\n)

[emanuelelongo]

for those coming here, the problem seems to be the differente end of line.
I solved like this:

var firepadRef = getExampleRef();
require.config({ paths: {'vs': '%PUBLIC_URL%/vs'}});
require(['vs/editor/editor.main'], function() {
    var model = monaco.editor.createModel("", "javascript");
     model.setEOL(monaco.editor.EndOfLinePreference.LF);
     var editor = monaco.editor.create(
         document.getElementById('firepad'), { mode } );
    Firepad.fromMonaco(firepadRef, editor)
});

[mikelehen]

cc/ @zach-karat @Progyan1997 who have been active in developing the monaco support and may be able to help find a fix to the underlying issue.

[zach-karat]

@mikelehen I agree with the fix posted above by Emanuele, we have something similar in our own code. I think it's basically a design choice whether the Monaco adapter should explicitly force a given line ending at setup time (or perhaps persist the line ending setting to Firebase in an auxillary reference so it may be shared between clients).

[mikelehen]

I see. At a high-level I would have hoped that your content would still be consistently synced between firepad instances, but you might end up with a mix of \r\n and \n newlines.

I wonder if it would make sense for firepad to force a uniform line-ending to automatically avoid this (maybe impossible if the mode has to be set when the editor is created), or else detect when it's not the desired line-ending and suggest / require you to change it.

[0xTheProDev]

It's usually left to implementation choice of the developer, you can always force it to use one of these.

Similar issue can also arise from renderFinalNewline constructor option, which is set to false for linux and true for Mac/Windows Ref

[zach-karat]

detect when it's not the desired line-ending and suggest / require you to change it

+1

[mikelehen]

FWIW, I wonder if this line is contributing to the problem: https://github.com/FirebaseExtended/firepad/blob/master/lib/monaco-adapter.js#L367

I guess it's assuming a uniform EOL character on every line in the file, which isn't actually the case... so either that line needs to be somehow fixed to respect the actual EOL character on each line, or we need to somehow guarantee a uniform EOL character across the file (either by detecting when it's "wrong" as I suggested above or performing some sort of normalization at some point).

[mrozekma]

I'm not sure the severity of this bug is appreciated -- it breaks cross-platform Monaco-backed Firepads completely, unless the users are willing to do everything on a single line. The fix described above doesn't seem to work for me; possibly Monaco changed since it was written.

I hacked together the following:

1. In Firepad, stop using monacoModel.getLinesContent() and switch to monacoModel.getValue(), a string containing the entire editor contents. This avoids the need to do this.lastDocLines.join(this.monacoModel.getEOL());, although the editors will still have different contents on different platforms.

2. In my app, set the default EOL style in Monaco. This has to be done before the model or editor are created, and which line ending doesn't really matter, as long as it's consistent across all users (edit: on second thought, using \r\n seems to be necessary, although I can't imagine why). I can't find any obvious public API for controlling this, so I had to resort to:

   import { StaticServices } from 'monaco-editor/esm/vs/editor/standalone/browser/standaloneServices';
   StaticServices.configurationService.get().updateValue('files.eol', '\r\n');

The reason setting the EOL on the model doesn't work is it's not a flag the model remembers; it's more like "convert the current contents to use this EOL", but ultimately Monaco uses a heuristic to determine what the EOL is by counting up how many lines end in each type of line ending. If you replace the contents with new data that has the wrong EOL, it will flip, and if you clear the contents it will resort to the default EOL (the value modified by step 2 above).

I don't know if this is enough for Firepad to fix this; you can do step 1 pretty easily, but I couldn't find a good way to do step 2 without poking data into a random service that they probably don't want user code messing with. It might be worth asking on the Monaco tracker

[samtstern]

@mrozekma thanks for this and you're right. I'm the current maintainer of the library but I often don't understand the severity of the issues since I was not one of the original authors and almost all of them have left Google since this lib was first published. At this point we really rely on the Firepad community to be the experts!

I agree that (2) is probably too hacky to be included in the library but if you can find a better way I'd be happy to work with you on a Pull Request.

[luminaxster]

This is STILL an issue. The TO logic should be not tied to SO variations.

If you run into it, this fix was last tested on Monaco ^0.40.0:

/// https://github.com/microsoft/monaco-editor/issues/144
// do no trust line endings from monaco editor API
const normalizeLineEndings = (str) => {
    return str?.replace(/\r\n|\r/g, "\n") ?? "";
}

//https://github.com/FirebaseExtended/firepad/blob/master/lib/monaco-adapter.js#L367
// fixes cross-platform line endings in monaco editor models on Windows and Linux by replacing them with \n only
// if they are not already \n.
const normalizeEditorModelText = (model, monaco) => {
    if (model.getEndOfLineSequence() === monaco.editor.EndOfLineSequence.LF) {
        return;
    }
    
    model.pushEOL(monaco.editor.EndOfLineSequence.LF);
    model.applyEdits([
        {
            range: model.getFullModelRange(),
            text: model.getLinesContent().reduce((r, e) => `${r}${normalizeLineEndings(e)}`, "")
        }
    ]);
}

// call it as soon a you create your model
const model = monaco.editor.createModel(text, language);
normalizeEditorModelText(model, monaco);

// IF YOU ALREADY HAVE CODE THAT NEEDS FIXING: CALL IT ON THE FIRST FIREPAD SYNC
firepadInstance.on('synced',...);
// FIXES ERRORS SUCH AS:
- Error: retain expects a positive integer.
- pushOperations() related errors coming from `MonacoAdapter.prototype.operationFromMonacoChanges`