bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

Severity: wishlist

Hello,

I've just tried to set a goal column (with "C-x C-n") and then scroll
through some buffer (which has text indented at different columns)
using the wheel of my mouse.

I expected that, while scrolling with the mouse, the cursor tried to
stay at the goal column I set.  That would have been helpful for
spotting those lines with that level of indentation.

But I've seen that the "goal column" feature seems to be limited only
to C-n and C-p (according to my test and the docstring of
"set-goal-column").

I think that it would make sense to extend this feature to other
vertical-scrolling commands like C-v, M-v, scroll-bar-toolkit-scroll
or mwheel-scroll.


In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
 of 2014-04-21 on LEG570
Repository revision: 117001 dancol@dancol.org-20140421012855-xu7gwqdl59pgkgur
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking 'CFLAGS=-O0 -g3' CPPFLAGS=-DGLYPH_DEBUG=1'

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

> But I've seen that the "goal column" feature seems to be limited only
> to C-n and C-p (according to my test and the docstring of
> "set-goal-column").
>
> I think that it would make sense to extend this feature to other
> vertical-scrolling commands like C-v, M-v, scroll-bar-toolkit-scroll
> or mwheel-scroll.

And BTW: C-n and C-p try to preserve the horizontal position of the
cursor even if there is no goal column set.  That is TRT do do, but
again: why limit that behavior to C-n/C-p?  I think it would also make
sense for the above-mentioned vertical scrolling commands (which
currently place the point at the first column after moving it to
another line).

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

> sense for the above-mentioned vertical scrolling commands (which
> currently place the point at the first column after moving it to
> another line).

page-up and page-down do preserve the column.  So it looks like the
problem is in *some* scrolling commands (maybe the scroll-bar ones?).


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

> page-up and page-down do preserve the column.

I've just tested them here again [1], and they don't preserve the
column for me.   I've tried with and without a goal column defined,
and in both cases the point is placed at the left margin whenever they
jump to another line.

>  So it looks like the
> problem is in *some* scrolling commands (maybe the scroll-bar ones?).

As I say, I can reproduce the problem with every vertical-scrolling
command I've tried (except C-p/C-n): scroll-up-command,
scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll.


------------
[1] From "emacs -Q", in:

  In GNU Emacs 24.4.50.1 (i686-pc-mingw32)
   of 2014-04-21 on LEG570
  Repository revision: 117001 dancol@dancol.org-20140421012855-xu7gwqdl59pgkgur
  Windowing system distributor `Microsoft Corp.', version 6.1.7601
  Configured using:
   `configure --enable-checking 'CFLAGS=-O0 -g3' CPPFLAGS=-DGLYPH_DEBUG=1'


-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

>> page-up and page-down do preserve the column.
> I've just tested them here again [1], and they don't preserve the
> column for me.   I've tried with and without a goal column defined,
> and in both cases the point is placed at the left margin whenever they
> jump to another line.

Indeed, I tested in an Emacs which had scroll-preserve-screen-position set.
Sorry,


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

Another issue related to the one reported originally: 'C-l'
(recenter-top-bottom) resets the goal column.

Steps to reproduce, from "emacs -Q":
  C-p C-p C-f C-n C-p

Note that the goal column (column 1) is honored by now.  Then do:
  C-n C-l C-p

Note that the goal column has been (incorrectly) forgotten after typing 'C-l'.


-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

> Another issue related to the one reported originally: 'C-l'
> (recenter-top-bottom) resets the goal column.
>
> Steps to reproduce, from "emacs -Q":
>   C-p C-p C-f C-n C-p
>
> Note that the goal column (column 1) is honored by now.  Then do:
>   C-n C-l C-p
>
> Note that the goal column has been (incorrectly) forgotten after typing 'C-l'.

I changed the severity of this bug from 'whishlist' to 'normal',
because of the above-quoted issue, which is related but different from
the original one.

Context information: I hit this bug frequently, whenever I have to
edit source code in a language (Informix 4GL) for which I've not found
an Emacs mode.  So, I simply edit those files in plain text mode, and
obviously in that scenario is very important that the current column
be preserved whenever possible, because that column marks the current
level of indentation.

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

>> Another issue related to the one reported originally: 'C-l'
>> (recenter-top-bottom) resets the goal column.
>> Steps to reproduce, from "emacs -Q":
>> C-p C-p C-f C-n C-p
>> Note that the goal column (column 1) is honored by now.  Then do:
>> C-n C-l C-p
>> Note that the goal column has been (incorrectly) forgotten after typing 'C-l'.

I think it would be fine to make C-l preserve `temporary-goal-column'.
Currently, only next-line and previous-line do that, and this is
hardcoded in `line-move' and `line-move-visual'.

We could change that hardcoded list by replacing it with a symbol
property `preserve-temporary-goal-column' and then add that property to
recenter-top-bottom.


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

On Thu, Oct 9, 2014 at 5:44 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> Another issue related to the one reported originally: 'C-l'
>>> (recenter-top-bottom) resets the goal column.
>>> Steps to reproduce, from "emacs -Q":
>>> C-p C-p C-f C-n C-p
>>> Note that the goal column (column 1) is honored by now.  Then do:
>>> C-n C-l C-p
>>> Note that the goal column has been (incorrectly) forgotten after typing 'C-l'.
>
> I think it would be fine to make C-l preserve `temporary-goal-column'.
> Currently, only next-line and previous-line do that, and this is
> hardcoded in `line-move' and `line-move-visual'.
>
> We could change that hardcoded list by replacing it with a symbol
> property `preserve-temporary-goal-column' and then add that property to
> recenter-top-bottom.

Sounds right to me.

Note also that, as I said before in this thread, any command intended
for _vertical_ motion of the cursor (scroll-up-command,
scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll, ...)
should try to preserve the goal column (whether semi-permanet or
temporary).

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

>> I think it would be fine to make C-l preserve `temporary-goal-column'.
>> Currently, only next-line and previous-line do that, and this is
>> hardcoded in `line-move' and `line-move-visual'.
>>
>> We could change that hardcoded list by replacing it with a symbol
>> property `preserve-temporary-goal-column' and then add that property to
>> recenter-top-bottom.
>
> Sounds right to me.
>
> Note also that, as I said before in this thread, any command intended
> for _vertical_ motion of the cursor (scroll-up-command,
> scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll, ...)
> should try to preserve the goal column (whether semi-permanet or
> temporary).

Therefore, `preserve-goal-column' would be a better name for the
property, since it would refer to both types of goal columns.

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

> Therefore, `preserve-goal-column' would be a better name for the
> property, since it would refer to both types of goal columns.

Actually both types are temporary (as opposed to `goal-column' which is
set typically once and for all by the major mode).


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

On Thu, Oct 9, 2014 at 6:57 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>> We could change that hardcoded list by replacing it with a symbol
>>> property `preserve-temporary-goal-column' and then add that property to
>>> recenter-top-bottom.
>> Sounds right to me.
>
> Patch welcome.

I currently lack the knowledge for making the change myself, sorry.

>> Note also that, as I said before in this thread, any command intended
>> for _vertical_ motion of the cursor (scroll-up-command,
>> scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll, ...)
>> should try to preserve the goal column (whether semi-permanet or
>> temporary).
>
> If you want that, just set scroll-preserve-screen-position accordingly.

I don't see how that would solve the problem I'm reporting.  For example:

* emacs -Q
* Visit the COPYING file from the Emacs tree.
* (setq scroll-preserve-screen-position t)
* M-m
* Scroll down with C-v until point falls on an empty line, so that the
  point can't stay at the original column.
* Now try to continue your scrolling down, but now with C-n.

Observe then how the original column is lost, which is IMO an annoying
bug which makes harder for me the analysis of tabulated files.

> I think there's a remaining bug in that the scroll commands will
> use their own "temporary goal-column".  So, for example, if you're on
> column 70, then do C-n to an empty line and then do page-down you'll end
> up in column 0 because page-down did not pay attention to
> temporary-goal-column (and vice-versa when switching from scrolling to
> C-n/C-p).
> Patch welcome to fix this as well.

Exactly.  That is what I'm trying to explain: All commands that move
point *vertically* to another line of text, either directly (like
C-p/C-n) or indirectly as consequence of scrolling the buffer (like
C-v/M-v) should share a single "temporary goal column", which is the
column where point was after the last non-vertical scrolling command.

>> Therefore, `preserve-goal-column' would be a better name for the
>> property, since it would refer to both types of goal columns.
>
> Actually both types are temporary (as opposed to `goal-column' which is
> set typically once and for all by the major mode).

I'm lost here.  I was aware of only these two types of "goal columns":

1. Temporary: Set after every command which moves point, except for
   those commands intended for _vertical_ motion (C-p/C-n/C-v/M-v/...).

2. Semi-permanent: Set with the `set-goal-column' command.  When this
   goal column is defined, it prevails over the temporary one.


-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

>> If you want that, just set scroll-preserve-screen-position accordingly.
> I don't see how that would solve the problem I'm reporting.  For example:

It solves the problem as long as you don't mix cursor-movement (where
you move point, which in turn might cause Emacs to move the viewport)
with scrolling (where you move the viewport, which might in turn cause
Emacs to move point).

>>> Therefore, `preserve-goal-column' would be a better name for the
>>> property, since it would refer to both types of goal columns.
>> Actually both types are temporary (as opposed to `goal-column' which is
>> set typically once and for all by the major mode).
> I'm lost here.  I was aware of only these two types of "goal columns":
> 1. Temporary: Set after every command which moves point, except for
>    those commands intended for _vertical_ motion (C-p/C-n/C-v/M-v/...).
> 2. Semi-permanent: Set with the `set-goal-column' command.  When this
>    goal column is defined, it prevails over the temporary one.

That's right.  `goal-column' is number 2.  Your problem is with
number 1.


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

>>> If you want that, just set scroll-preserve-screen-position accordingly.
>> I don't see how that would solve the problem I'm reporting.  For example:
>
> It solves the problem as long as you don't mix cursor-movement (where
> you move point, which in turn might cause Emacs to move the viewport)
> with scrolling (where you move the viewport, which might in turn cause
> Emacs to move point).

Right, and I find that behavior clearly undesirable.  The
temporary goal column should not be (re)set by any of those
vertical-motion commands.

-- 
Dani Moncayo

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

>>>> If you want that, just set scroll-preserve-screen-position accordingly.
>>> I don't see how that would solve the problem I'm reporting.  For example:
>> It solves the problem as long as you don't mix cursor-movement (where
>> you move point, which in turn might cause Emacs to move the viewport)
>> with scrolling (where you move the viewport, which might in turn cause
>> Emacs to move point).
> Right, and I find that behavior clearly undesirable.

I don't doubt it and I agree it's a bug.


        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Stefan Monnier]

>> We could change that hardcoded list by replacing it with a symbol
>> property `preserve-temporary-goal-column' and then add that property to
>> recenter-top-bottom.
> Sounds right to me.

Patch welcome.

> Note also that, as I said before in this thread, any command intended
> for _vertical_ motion of the cursor (scroll-up-command,
> scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll, ...)
> should try to preserve the goal column (whether semi-permanet or
> temporary).

If you want that, just set scroll-preserve-screen-position accordingly.
Maybe we should change this default value.  I'd suggest you bring it up
on emacs-devel to see what other people think about it.

I think there's a remaining bug in that the scroll commands will
use their own "temporary goal-column".  So, for example, if you're on
column 70, then do C-n to an empty line and then do page-down you'll end
up in column 0 because page-down did not pay attention to
temporary-goal-column (and vice-versa when switching from scrolling to
C-n/C-p).
Patch welcome to fix this as well.



        Stefan

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Lars Ingebrigtsen]

Dani Moncayo <dmoncayo@gmail.com> writes:

> Note also that, as I said before in this thread, any command intended
> for _vertical_ motion of the cursor (scroll-up-command,
> scroll-down-command, scroll-bar-toolkit-scroll, mwheel-scroll, ...)
> should try to preserve the goal column (whether semi-permanet or
> temporary).

(I'm going through old bug reports that unfortunately weren't resolved
at the time.)

It looks like this would be pretty easy to implement, but it would be a
behavioural change -- and the goal column is explicitly documented to
work on C-n/C-p commands, and not the other commands.

On the other hand, I think anybody's who has done a `C-x C-n' would want
<prior> and <next> to also respect the goal column, really.  So I think
I'd be in favour of just changing the behaviour here.

Does anybody else have an opinion?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Drew Adams]

> On the other hand, I think anybody's who has done a `C-x C-n' would
> want <prior> and <next> to also respect the goal column, really.

Why would you assume that?

> So I think I'd be in favour of just changing the behaviour here.
> 
> Does anybody else have an opinion?

Can users not, themselves, get this behavior for
anything they like?  Can't they define a command
that does what `scroll-up-command` (or whatever)
does but also respects the goal column?

If you want to provide a user options that makes
some particular set of commands respect the goal
column, then do so.  The option value could be a
list of such commands.

Alternatively, users could maybe put some property
on the command symbol to indicate this.

However you might want to implement such a feature,
it should (1) be opt in and (2) let users control
the set of affected commands.

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> It looks like this would be pretty easy to implement, but it would be a
> behavioural change -- and the goal column is explicitly documented to
> work on C-n/C-p commands, and not the other commands.
>
> On the other hand, I think anybody's who has done a `C-x C-n' would want
> <prior> and <next> to also respect the goal column, really.  So I think
> I'd be in favour of just changing the behaviour here.
>
> Does anybody else have an opinion?

I've now made this change in Emacs 29, and I have not added a new user
option to tweak the behaviour.  If there's a huge backlash, we can add
one, but it seems rather unlikely to me because of the reason above.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Drew Adams]

> I've now made this change in Emacs 29, and I have not added a new user
> option to tweak the behaviour.  If there's a huge backlash, we can add
> one, but it seems rather unlikely to me because of the reason above.

The doc of `(next|previous)-line' specifically calls
out the behavior wrt `goal-column' (see below).

Are you doing that also for each command/key you
change now to respect `goal-column'?  You should, IMO.

Better would be to define a minor mode, which would
cause some set of commands to respect `goal-column'.
This is really a modal thing and not part of the
behavior of this or that particular command.

The set of affected commands could be defined as a
list-valued option or by putting a property on a
command's symbol.  One way or another, this should
be easily under user control.

The doc of the minor mode would document the
behavior as well as point to the set of affected
commands.

No need then to also put that info in the doc of
the individual commands.  (Users's then wouldn't
see it in `C-x k' or `C-x f', but they'd see it in
`C-h m', and of course it only takes effect after
`C-x C-n' confirmation anyway.)

`C-x C-n' could (after novice confirmation for
setting the goal column) turn on the minor mode,
which would otherwise be off by default.  Its doc
should also mention that minor mode behavior.
___

Current doc about this in `(next|previous)-line'
(it doesn't really belong there):

 The command C-x C-n can be used to create
 a semipermanent goal column for this command.
 Then instead of trying to move exactly vertically (or as close as possible),
 this command moves to the specified goal column (or as close as possible).
 The goal column is stored in the variable 'goal-column', which is nil
 when there is no goal column.  Note that setting 'goal-column'
 overrides 'line-move-visual' and causes this command to move by buffer
 lines rather than by display lines.
___

(BTW, that part of the doc isn't filled properly.)

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Michael Welsh Duggan]

Dani Moncayo <dmoncayo@gmail.com> writes:

> I've just tried to set a goal column (with "C-x C-n") and then scroll
> through some buffer (which has text indented at different columns)
> using the wheel of my mouse.
>
> I expected that, while scrolling with the mouse, the cursor tried to
> stay at the goal column I set.  That would have been helpful for
> spotting those lines with that level of indentation.
>
> But I've seen that the "goal column" feature seems to be limited only
> to C-n and C-p (according to my test and the docstring of
> "set-goal-column").
>
> I think that it would make sense to extend this feature to other
> vertical-scrolling commands like C-v, M-v, scroll-bar-toolkit-scroll
> or mwheel-scroll.

Try setting scroll-preserve-screen-position.
(setq scroll-preserve-screen-position t)

-- 
Michael Welsh Duggan
(md5i@md5i.com)

bug#17346: 24.4.50; Why is the goal column limited to C-n and C-p ?

[Dani Moncayo]

> Try setting scroll-preserve-screen-position.
> (setq scroll-preserve-screen-position t)

Thanks, but that is to the behavior I want, and it wouldn't solve the
problem I'm reporting: any command intended to move point _vertically_
should try to place the point at the "goal column" if there is one
currently defined, or else at the column of its previous location (so
that the movement be indeed "vertical").  The problem is that only C-p
and C-n seem to behave that way.

-- 
Dani Moncayo