Update CodeMirror

[marcelgerber]

For #12031.

Several tests fail, but most of them do fail on master, too, so I suppose it's not my fault.
I, though, had to disable 3 EditorCommandHandlers tests which have to do with how CM handles atomic markers mid-text. There was a slight behavior change in codemirror/codemirror5@1fae537 which makes these tests fail.
This change could theoretically affect inline editors, but in my manual testing I didn't face any issues.
For more reference on this change, see codemirror/codemirror5#3699.

Fixes #11905, #11683, and a whole lot more.

[ficristo]

@marcelgerber++ 👍

[abose]

@marcelgerber 👍

[abose]

Tagging @swmitra

[MiguelCastillo]

@marcelgerber I know these tests failed before your update. But do you think we can fix them in this PR to make sure we are not introducing regressions? It would awesome to have peace of mind.

[MiguelCastillo]

It would be nice to explicitly call out which version of CodeMirror we are updating to. :)

[MiguelCastillo]

@marcelgerber I found the particular change in CodeMirror that is causing this regression. I am going to dig in a little bit more to see if I can find the reason why the change was made. The question is whether or not that is the correct behavior in CodeMirror.

codemirror/codemirror5@1fae537?diff=unified

[MiguelCastillo]

Well, this thread has the history of why the change of was made. codemirror/codemirror5#3658

[MiguelCastillo]

@marcelgerber I found what we have to change on our side. The changes in codemirror actually seems to make things more consistent in behavior. I am still running tests.

https://github.com/adobe/brackets/blob/master/src/editor/Editor.js#L1507
That line would change to {line: to - 1, ch: 0}. We don't need to figure out the length anymore; CodeMirror will take the whole line.

[MiguelCastillo]

@marcelgerber - BTW - if you want to see a visual of the behavior:

1 go to https://codemirror.net/index.html
2 open the browser console. We are going to execute the following commands

visibleRange1 = editor.markText({line: 0, ch: 0}, {line: 4, ch: 0}, {collapsed: true, inclusiveLeft: true, inclusiveRight: true, clearWhenEmpty: false});

visibleRange2 = editor.markText({line: 5, ch: 0}, {line: 8, ch: 0}, {collapsed: true, inclusiveLeft: true, inclusiveRight: true, clearWhenEmpty: false});

3 Select anything spanning multiple lines.

editor.setSelection({line: 0, ch: 0}, {line: 8, ch: 0});

4 Now clear both markers to see the selected text in the editor.

visibleRange1.clear();
visibleRange2.clear()

5 Checkout the selected range.

editor.doc.sel

That selects the whole line regardless of what's around, which is the behavior we want when we are hiding lines as the unit test does.

[MiguelCastillo]

I found an odd behavior in CodeMirror that is causing our particular unit test failures. I have provided information over on the CodeMirror side.

[MiguelCastillo]

@marcelgerber I think the issue is fixed in the latest code mirror. https://github.com/codemirror/CodeMirror/releases/tag/5.13.0. Gotta bring it down and test it.

[Denisov21]

Now is exit a new version of Code Mirror https://github.com/codemirror/CodeMirror/releases/tag/5.13.2

[marcelgerber]

Thanks for pointing that out!
I've updated to the latest version and now these tests, after being re-enabled pass again.

[MiguelCastillo]

@marcelgerber CodeMirror had a couple of changes around ranges that fixed the issue :)

[MiguelCastillo]

I pulled your branch. I am running today with it and if things go well, we can merge?

[marcelgerber]

Surely we can!
I've just now squashed the commit to get them down to only 2.

[MiguelCastillo]

things seem to work ok. I will merge now so that more people can get to test this update early. Thank you @marcelgerber !!