@marcelgerber++ 👍

@marcelgerber 👍

Tagging @swmitra

@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.

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

@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

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

@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.

@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.

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

@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.

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

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

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

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

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

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