Fix #10391 - incorrect cursor position after autocomplete

[dmonad]

References

This PR fixes the issue described in #10391.

Code changes

The auto-complete plugin now explicitly sets the cursor position after the replacement.

Previously, we relied on the behavior of modeldb to adapt the cursor position correctly. The autocomplete plugin simply replaced text and the local position would be automatically updated to the position after the replacement. This is no longer possible in all cases because y-codemirror now updates the cursor positions after each change with an algorithm that is designed to work nicely with remote changes. We instead update the position to the position before the replacement, because that will lead to fewer conflicts when multiple peers insert content at the same time.

User-facing changes

None

Backwards-incompatible changes

None

[jupyterlab-dev-mode]

Thanks for making a pull request to JupyterLab!

To try out this branch on binder, follow this link:

[blink1073]

Thank you!

[krassowski]

Thank you!

[dmonad]

Thanks for the quick review! I fixed the tests, hope they run now.

[krassowski]

I don't think the fix works now. The dict.<tab> example gives me dict.|clear again. What was the sequence the tests were failing on?

Edit: give me a minute to confirm, maybe I had stale binder 👀

[krassowski]

Yup, not working after the changes that fixed the test:

Debug info:

patch = {start: 5, end: 5, value: "clear"};
cursorBeforeChange = 5

this means that the condition of cursorBeforeChange < end && cursorBeforeChange >= start will not be true and the fix will not be applied. In fact I, wonder if this condition will ever be true (that would require that cursor is simultaneously before end and after start which should never be true for well behaved patches?) - did you mean to make the condition the other way round? It would help if you add a comment on why this was needed.

Also, maybe it would be useful to add a test for dict.|<tab> -> dict.clear|?

[dmonad]

Thanks @krassowski for trying out the patch! I fixed the issue on by including the end as part of the range. I also added a test for autocompletion on an empty work (replace "" with "foobar" for example, which should cover the cases described in the original issue).

that would require that cursor is simultaneously before end and after start which should never be true for well behaved patches?

When performing autocomplete on the empty word (e.g. "" ⇒ "dict"), the cursor is within the replaced range. Although in this case start == end. Usually, the cursor would be after the replaced range (e.g. "di|" ⇒ "dict|").

This PR only fixes an edge case. When the cursor is within the replaced word (or directly after it), we expect that the cursor is after the replaced word after the change is performed. All other cases are handled by the existing cursor mapping approach.

[dmonad]

I rebased because the tests were failing. Now different tests that are unrelated to this PR are failing. I'm going to try rebasing again tomorrow.

[blink1073]

@meeseeksdev please backport to 3.1.x