Set selection in Move Line Up after operation is finished

[njx]

Fixes #1343.

I believe the issue is that the actual edit is happening in the underlying "document" (which is actually the full editor), but the selection is being set in the inline editor (which is a different editor). So the selection is getting set in the inline editor, but then the change is synchronized back from the document when the operation ends, which modifies the selection we just set. The inline editor selection change really shouldn't be inside the document's batch.

So the fix is just to move the selection change (in the inline editor) outside of the operation.

[njx]

Actually, this is an okay fix (there's no CodeMirror bug). Updated description to explain why.

[peterflynn]

Do you think we should add a comment explaining why the selection change shouldn't be inside the batch? I think normally the best practice is to do everything in the batch, including selection changes, to avoid extra DOM updates.

Looks good otherwise.

[peterflynn]

And actually, looks like there are several other places in EditorCommandHandlers alone where we set selection inside a batch. Do any of those have a similar bug? I'll try to do some testing...

[njx]

Yup, good idea. Done.

[peterflynn]

Yup, looks like block comment, at least, suffers from the same thing:

1. Open a JS function in the inline editor
2. Select three lines of its body
3. Ctrl+Shift+/

Result: selection is shifted down one line from where it should be.

Seems like we might need to do an audit of setSelection() calls to ferret out all cases of this... is that worth doing now?

[njx]

Yup, I just reproduced something similar for line comments as well. I'll look at fixes for these. I could imagine building some infrastructure around this (e.g. having editor.setSelection() detect whether its underlying document is in a batch, then delaying the selection setting until after it gets the next synchronize), but it doesn't seem worth doing that in advance of the doc/view split work, so I think we should just fix these individually for now.

[njx]

Other fixes pushed. EditorCommandHandlers unit tests still pass.

[TomMalbran]

I just checked this and it seems like line comment in CSS also has this problem when the selection ends on a ch 0 selecting content in at least 2 lines.

[njx]

@TomMalbran - do the fixes I just pushed to this pull request fix the issue you're seeing?

[TomMalbran]

I believe they will, since is the same problem as the ones before... but haven't tested it.

[peterflynn]

Looks good -- merging