Fix textLinesMutator when dealing with insertions/deletions at the end of pad

[webzwo0i]

Will clean this up tomorrow (fixing eslint etc). This should fix https://github.com/ether/etherpad-lite/issues/5214 https://github.com/ether/etherpad-lite/issues/5213 and replace https://github.com/ether/etherpad-lite/pull/5215 and https://github.com/ether/etherpad-lite/pull/2911

Also adds some comments and additional coverage.

[webzwo0i]

As a side note: The commits that fixed https://github.com/ether/etherpad-lite/issues/2802 should be re-evaluated.

[webzwo0i]

This should be ready now.

[webzwo0i]

Thanks for the input @rhansen! Will clean up.

Regarding putting the test into the same commit as the fix, I thought we also put the test in a separate commit (before all other commits) as stated somewhere in CONTRIBUTING.md. But in this case, as it's fixing multiple bugs in one PR, I'll definitely include them into the correct commit.

[rhansen]

Rebased onto latest develop.

[rhansen]

Regarding putting the test into the same commit as the fix, I thought we also put the test in a separate commit (before all other commits) as stated somewhere in CONTRIBUTING.md.

It's OK to add an xit() test before the commit that fixes the bug, then change the xit() to it() in the commit that fixes the bug. I generally don't like that approach for a couple of reasons:

• It disconnects the addition of the new test from the reason the new test was added (as a regression test for a bugfix). This makes it harder for reviewers to determine that the new test is actually a proper regression test for the bugfix.
• Mocha doesn't attempt xit() tests. Other test frameworks support the concept of "expected to fail" tests that are still run but don't result in overall failure unless they actually pass (in which case the "expected to fail" should be converted to the usual "expected to pass").

[rhansen]

@webzwo0i I rebased onto latest develop and made some changes to address my feedback. The "easysync tests: move to separate files" commit was split into 5 different commits to make it easier to review. I think the only thing left is to add the regression tests to the commits that fix the associated bugs.

[webzwo0i]

Thanks! I added the tests to the appropriate commits, added poolOrArray to easysync-assembler and also refined the commit messages of the last three commits.

Rebased onto latest develop

[webzwo0i]

OK, I think I ensure that newlines are everywhere first. It's possible that I didn't saw missing newlines because it's dealing with edits at the end of the pad.

[rhansen]

Any update @webzwo0i?

[webzwo0i]

Will have time in the next few days. I'll probably tackle curCol/curLine behavior first, including asserting the invariants. For newlines behavior I need to take a look at mutateAttributionLines

[rhansen]

I rebased onto latest develop and pushed a couple of fixup commits.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[LeaChemoul]

Any update on thoses fixup commits ?

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[JohnMcLear]

Bump @webzwo0i -- any progress?

[JohnMcLear]

@dependabot rebase

[SamTV12345]

@webzwo0i Do you have some time to fix this pr?