On Friday, October 04, 2013 05:31:20 AM Philip Peitsch wrote:
Hi all,
Just quickly brain-dumping some recommendations for how to improve the clarity and reduce terminology confusion within SelectionMover.
Some commonly repeated terms that appear badly overloaded in SelectionMover are steps, positions and points.
Add to this, the functions available are also extremely confusing both in terms of their names, and the internal code (there is a patch coming today to help with the code side).
So, new terms of reference I'm standardising on are:
Points ====== Raw DOM positions. This is any valid node & offset pair.
Related functions ----------------- SelectionMover.StepCounter.countStepsToPosition - converts the distance between two Points into to it's (roughly) equivalent number of Steps
Positions ========= A set of positions as returned by the position iterator. This is a subset of points as these can be filtered at the node level via a NodeFilter.
Related functions ----------------- SelectionMover.StepCounter.countForwardSteps SelectionMover.StepCounter.countBackwardSteps - converts the number of Steps into it's equivalent number of Positions
SelectionMover.movePointForward SelectionMover.movePointBackward - Move the cursor the requested number of Positions in forward or backwards
Steps ===== A filtered subset of positions as returned by the position iterator. This is controlled using a PositionFilter.
Related functions ----------------- SelectionMover.StepCounter.countStepsToPosition - converts the distance between two Points into to it's (roughly) equivalent number of Steps
SelectionMover.StepCounter.isPositionWalkable - returns true if the supplied iterator is a valid Step
SelectionMover.StepCounter.convertForwardStepsBetweenFilters SelectionMover.StepCounter.convertBackwardStepsBetweenFilters - translates between one filtered set of Steps to a different filtered set of Steps. IMPORTANT: this function will only work filter1 only accepts a subset of filter2.
countStepsToLineBoundary - convert the distance between the cursor and nearest line boundary into Steps
countLinesSteps - convert the distance between the cursor and the same horizontal position X lines up/down into Steps
My recommendations (I'm assuming you want to hear these :) )
1. Standardise SelectionMover on Points and Steps only. Any attempts to move a cursor to a Position that is not also a Step is likely a bug. The only place where we deliberately move in non-Step increments is when trying to put the cursor back into a filtered Step. This same effect could be achieved by calling moveForwardStep(0, filter).
100% Agreed.
2. Remove or rewrite convert(Forward|Backward)StepsBetweenFilters. The current implementation will not work for arbitrary filter pairs. These should likely be re-written to return the number of filter1-steps when travelling filter2-steps in distance. I would also suggest this takes the same approach as countStepsToPosition and round the cursor back to the Point of the last filter2-step that is smaller-than or equal-to the current cursor Point.
Good point; at the time when I wrote those I was only looking at a specific use-case - mapping from TextPositionFilter-space to (TextPositionFilter & RootFilter)-space, and that approach subconsciously manifested itself here. Agree that this should be made to work with *really* arbitrary filter pairs. In all, thanks for the detailing. We should put this in a README- SelectionMover.txt. Cheers, Aditya