[WebODF] SelectionMover and standardising lingo

[Aditya Bhatt]

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