This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Repository navigation
This repository was archived by the owner on Sep 6, 2021. It is now read-only.
Move line up/down has incorrect behavior in edge cases in inline editor #3130
Description
- Open the Getting Started project
- Put the cursor in the tag
- In the inline editor, put the cursor inside the comment ("/* hide samp...")
- Edit > Move Line Down until it gets to the bottom
- Put the cursor in the last line (the brace)
- Edit > Move Line Up
Result:
- In (4), the line can't be moved to the last line of the editor.
- In (6), the editor shrinks, hiding the line below the brace.
Activity
njx commentedon Mar 14, 2013
These are leftover issues after the fix for #1933. I don't think they're very high priority--you probably usually don't want to move a line outside of the end of the rule/function anyway (or move the end line above other content). But it feels asymmetric since the same issues don't occur at the top; it's almost like we should consider actually making the same behavior happen at the top. Also, in the future we might have some other kinds of inline editors for which this behavior would be less sensible.
Marking low priority.
TomMalbran commentedon Mar 14, 2013
The problem here is that moving the line down to the last line in the inline editor triggers the special case in
TextRange.prototype._applySingleChangeToRange, and closes the editor, which I found out that is the right call. So to get a better fix it would require to handle this special case and be able to move down every time. Or it would be easy enough to make it asymmetric.njx commentedon Mar 18, 2013
@TomMalbran - I haven't had a chance to dig into your code, but I think the general issue is that the TextRange is very sensitive to whether a replacement crosses the boundary. In the Move Line Down code, does it replace the
\nat the end of the line below? If it does, a fix might be to make it so that in this special case, you change things around so it replaces the\nat the beginning instead, or something similar.njx commentedon Mar 18, 2013
Reviewed--assigning to @TomMalbran to look at.
jasonsanjose commentedon Apr 17, 2013
Confirmed fixed. Auto closed by pull #3233.