Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document.remove() removes lines in backwards order #1211

Closed
ggoodman opened this issue Jan 17, 2013 · 2 comments
Closed

Document.remove() removes lines in backwards order #1211

ggoodman opened this issue Jan 17, 2013 · 2 comments

Comments

@ggoodman
Copy link
Contributor

Because the order of deletion is last, middleLines, first, the change events that get fired are in reverse order. If you watch those change events, the attached Range objects do not contain relevant ranges.

The three blocks in the linked lines should be reversed:

https://github.com/ajaxorg/ace/blob/master/lib/ace/document.js#L436-L445

@nightwing
Copy link
Member

What do you expect them to be?
for document

a0
1
2
3a

when removing number i get

{ action="removeText", range=Range: [3/0] -> [3/1], text="3"}
{ action="removeLines", range=Range: [1/0] -> [3/0], }
{ action="removeText", range=Range: [0/1] -> [0/2], text="0"}
{ action="removeText", range=Range: [0/1] -> [1/0], text="\n"}

with reversed order it would become

{ action="removeText", range=Range: [0/1] -> [0/2], text="0"}
{ action="removeLines", range=Range: [1/0] -> [3/0], }
{ action="removeText", range=Range: [1/0] -> [1/1], text="3"}
{ action="removeText", range=Range: [0/1] -> [1/0], text="\n"} // this can't be moved!

which is hardly any better.

Though it might be good to simplify deltas and have only insert and remove.

@ggoodman
Copy link
Contributor Author

Thanks for responding @nightwing, you are right that the ranges appear correct after all and the issue was with my translation of those ranges to text offsets.

As for josephg/ShareJS#159 I'm not sure what the issue would be after all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants