On 26/06/2013, at 11:56 PM, Friedrich W. H. Kossebau wrote:
Hi,
when testing if there will be any regressions on always normalizing* the position and length in OpRemoveText I found that there is different behaviour if in the following structure the char is removed by a backspace or a delete:
Currently these are the results:
... on a backspace from behind the "A"
(ignore the cursor position, different topic, handled in separate email)
... on a delete from before the "A"
I would have expected to get the same results in both cases.
+1 for this. This is definitely a side-effect, not an intended thing
Playing with LO I found that there the span is kept in both cases (tested by creating an empty document, inserting an "A", marking it and directly formatting it to bold, removing the A in either ways again and saving as .fodt and inspecting with a text editor, while also seeing that with the empty document the Bold is toggled and any inserted chars are ending up in the span, being formatted with Bold). … [snip] What do you think? Should emptied spans be deleted in this case? Or should empty spans only be deleted once a deletion range crosses their border?
Ideally the deletion should only happen once the range crosses the node's boundary. This is going to be tricky however as currently OpRemoveText is REALLY text oriented, and really will need to be re-written to be able to handle the (imo correct) behaviour you're proposing. We'll need to rewrite it anyways, as a the moment it is unable to remove images, tables, etc. and is not really terribly reliable on complex documents. I can also see no easy way to adapt the existing code easily to accommodate this, as the whole operation is driven by text neighbourhoods. Hence the recent tab deletion patch, it was easier to ADD text for the operation to remove. (No criticism intended of the original code btw… it has served it's purpose well so far :-), I just think we're now rapidly out-growing it!) I have some ideas about using position iterators and ranges, but similar to a top-secret military-funded research project, I currently have no functional implementations ;-). The basic intent is this becomes OpRemoveRange instead, and is focused primarily on removing "valid" ODT positions in the supplied range.