[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point
From: |
Drew Adams |
Subject: |
bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point |
Date: |
Tue, 28 Feb 2017 07:11:08 -0800 (PST) |
> > 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.
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, (continued)
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Drew Adams, 2017/02/19
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, npostavs, 2017/02/26
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Drew Adams, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, npostavs, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Drew Adams, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Noam Postavsky, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Drew Adams, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Noam Postavsky, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, Drew Adams, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point, npostavs, 2017/02/27
- bug#25777: 25.1; [PATCH] `rectangle--pos-cols' should not move point,
Drew Adams <=