yjs / y-codemirror.next

Collaborative extensions for CodeMirror6
Other
159 stars 38 forks source link

Inconsistent positions/ranges for files that contain `\r\n` as a line separator. #35

Open DanielCodesphere opened 2 months ago

DanielCodesphere commented 2 months ago

Describe the bug Codemirror treats the line separator \r\n as a single character which means ranges and positions are different from yjs. If a file contains \r\n then new characters written after it appear correctly in codemirror but are actually added at the wrong position in the yjs document. Also adding text directly to the end of the yjs document throws an error because the change will be out of range for codemirror.

It seems like \r\n being only one character is intended behavior for codemirror and positions should be manually adjusted.

To Reproduce Steps to reproduce the behavior:

  1. Create a file containing 1\r\n2 where \r\n is a line separator, not normal text.
  2. Write another character at the end, it will appear as 1\r\n23 in codemirror but 1\r\n32 in yjs.
  3. Inserting a character at the end of the yjs doc throws a range error when the change is dispatched here.

Expected behavior Ranges and positions are always consistent between yjs and codemirror.

Environment Information

dmonad commented 2 months ago

Why don't you simply use \n, so you have consistent content across platforms? Are you saying that windows doesn't display codemirror content correctly?

DanielCodesphere commented 2 months ago

I haven't tested it on windows but i would assume that the problem is the same. Because \r\n is one character in codemirror and two characters in yjs they don't line up which causes the problems i described in the bug report.

For user files i would prefer if i didn't have to replace \r\n with \n since that might cause issues if they later try to open the file in another editor on windows again.

dmonad commented 2 months ago

If you use Yjs for collaborative editing, then you probably expect that all users (probably on different platforms) end up with the same content.

It is not entirely clear to me why one would choose to use \r\n on some platforms and \n on the others.

I need good arguments to make good decisions. A fix for this codemirror-specific issue is quite complicated to implement and will have impact on other editor bindings. I'm currently working under the assumption that it would be preferable for everyone to use the same line-ending format.

If you store files on the filesystem, I suggest that you manually transform the \n to \n\r on windows and that you transfer it back before you intert the content into codemirror.

Since I don't understand the issue, it will probably be stalled until I do.

DanielCodesphere commented 2 months ago

Codemirror supports three different line separators: \n, \r and \r\n. Both \n and \r work fine with this extension because they are only one character.

The problem i have is that a user can open a file they have created using a different editor that uses \r\n as a line separator. Initially that is not a problem, all collaborators see the same file with the same line separators rendered correctly. But as soon as someone types something, that character will be added to the wrong position in the yjs doc, no matter what platform they are on.

A way around that would be to replace all \r\n with just \n. But that might cause issues later on if the user then saves the file and opens it with a different editor, because the line separator has now changed. So i would have to remember what line separator was used initially and then transform it on every save/load. This could work, however it is still just a workaround for documents containing \r\n breaking this extension.

I don't see why fixing this would impact other editor bindings, because it should just slightly adjust the positions reported by codemirror and the positions when yjs changes are dispatched to codemirror. So it should only affect this extension.

dmonad commented 2 months ago

As I said, the issue is quite complicated to fix and will have impact on performance.

My point stands, I don't see any use-case for using different line breaks in a collaborative environment. A Linux user would insert \n and a Windows user \r\n. In collaborative environments we have to expect that Windows users are working on the same document as Linux users. It would be a very bad idea to mix different kinds of line endings.

I could replace Windows line endings with normal ones before I insert them in the Yjs structure. codemirror would render \r\n only if EditorState.lineSeparator is set. Then Windows users would render \r\n consistently and Linux users would render \n consistently. But this is probably not the solution you are looking for.

In any case, I will put this on the back burner as I don't deem this a high-priority issue. Feel free to contact me if you want to sponsor work on this.

DanielCodesphere commented 2 months ago

I still don't think the os actually matters and it is also not an issue of mixing different line breaks. It's just that as soon as the yjs document contains \r\n this extension breaks, no matter the platform.

But i do understand that if this is difficult to fix, it's probably not worth it, since it can still be worked around.