bug#3494: Review line-move-visual patch

bug#3494: Review line-move-visual patch

[Chong Yidong]

Hi Stefan,

I just checked in a patch into the trunk, which makes line-move-visual
play properly with truncated lines (Bugs 3494 and 3805).  I think it
should go to the branch as well.  Could you review it?

2009-07-11  Chong Yidong  <cyd@stupidchicken.com>

	* simple.el (temporary-goal-column): Change the value for
	line-move-visual to a cons cell.
	(line-move-visual): Record or set the window hscroll, if
	necessary (Bug#3494).
	(line-move-1): Handle cons value of temporary-goal-column.

bug#3494: Review line-move-visual patch

[Stefan Monnier]

> I just checked in a patch into the trunk, which makes line-move-visual
> play properly with truncated lines (Bugs 3494 and 3805).  I think it
> should go to the branch as well.  Could you review it?

> 2009-07-11  Chong Yidong  <cyd@stupidchicken.com>

> 	* simple.el (temporary-goal-column): Change the value for
> 	line-move-visual to a cons cell.
> 	(line-move-visual): Record or set the window hscroll, if
> 	necessary (Bug#3494).
> 	(line-move-1): Handle cons value of temporary-goal-column.

The problem with this code is that vertical-motion is defined as:

   LINES can optionally take the form (COLS . LINES), in which case
   the motion will not stop at the start of a screen line but on
   its column COLS (if such exists on that line, that is).

but without specifying what is meant by COLS w.r.t hscroll.
Similarly the docstring refers to "start of the screen line" without
making it clear what it means in the case of hscroll.

So, I'm not sure if doing it like you've done is right or not.
Maybe a better solution is to use COLS + HSCROLL as the
temporary-goal-column"; it would at least avoid having to change
temporary-goal-column to accept a cons cell rather than only a number.



        Stefan