bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point

[Drew Adams <drew.adams@oracle.com>]

> > I think it's great that a function such as `rectangle--pos-cols'
> > was added.  (But it has no business being considered "internal".)
> >
> > Do you really think that a function whose only purpose is
> > to let you know what the rectangle columns are should move
> > point and leave it in a position that is not one of the
> > rectangle corners?
> 
> After looking over this thread again, I conclude we got totally
> side-tracked on the question of whether this is a regression or not.
> That's just a distraction.

I agree, and that was my fault.  In my mind it caused my code
to regress.  My bad.

> I think it's reasonable to make rectangle--pos-cols preserve point.
> Could you update your patch to
> 
> 1. Use a single save-excursion around the whole body, instead of adding
> several.
> 2. Rename rectangle--pos-cols to rectangle-pos-cols.
> 3. Add a docstring to rectangle-pos-cols.

I could do that, yes.

But IMHO, it is generally better to scope a `save-excursion'
as tightly as possible around the movement that you want to
control (hide, erase, undo).

Unless there is something critical for performance (and I
don't think `save-excursion' is costly), that's better.

Why?  Because it makes the code much clearer.  It tells you
that outside the `save-excursion' zones point is unlikely
to be moved.  And that makes maintenance easier and less
error-prone.

Putting a `s-e' at a wider location is analogous to putting
a mutex block at an unnecessarily wide location.  (Yes,
there is no real connection between those two, but it comes
down to doing something only where/when it's needed.)

If you confirm that you really want that wider scope here
for `s-e' then I'll do that.  Otherwise, I'll keep the
`s-e' occurrences where they are but do the renaming and
add a doc string.  Let me know.  Thx.