bug#20663: page.el (forward-page): Avoid skipping pages

[Marcin Borkowski]

On 2016-04-16, at 11:26, Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Marcin Borkowski <mbork@mbork.pl>
>> Cc: ambrevar@gmail.com, 20663@debbugs.gnu.org
>> Date: Wed, 13 Apr 2016 19:53:31 +0200
>> 
>> > Can one of you please explain why the original code misbehaves?
>> 
>> Quoting from the original report:
>> 
>> --8<---------------cut here---------------start------------->8---
>> > Running `emacs -Q example.txt`:
>> >
>> >    M-<
>> >    C-x n p
>> >    M->
>> >    M-1 C-x n p
>> >
>> > This should bring us from page 1 to page 2, but page 3 gets displayed 
>> > instead.
>> --8<---------------cut here---------------end--------------->8---
>
> Yes, I've read that.  I asked for an explanation of _why_ the code
> currently in Emacs misbehaves in this recipe.  Can one of you describe
> that?

Let me try (note: this is partly my conjecture!).

Since the concept of a "page delimiter" is vague in Emacs (the manual
suggests that a page delimiter is a _line_ such that `page-delimiter'
matches at its beginning, common sense suggests that it is a _substring_
of the buffer matching `page-delimiter', it's probable that someone
decided that if the point is at the beginning of a line and matches
`page-delimiter', then `forward-page' shouldn't just move past the text
matching `page-delimiter' we are on, but the next one.  So the author of
the current version of `forward-page' decided just to move one character
forward (in case we are at the line beginning), so that `page-delimiter'
won't match.

This I consider a Bad Idea™.  As I mentioned earlier, I think the
concept of a "page" and "page delimiter" should be made more precise;
then, it should be enough to correct `forward-page' (basically all other
functions in page.el depend on it, directly or not).  Currently Emacs
seems to treat the _line_ as the page delimiter, but as the OP noted,
this yields strange/unintuitive results with narrowing.

And by the way, the patch the OP gave is also wrong, though in
a different way.  (I should have noticed that earlier.)  The OP proposed
this instead of (if (bolp) (forward-char 1)):

(if (string= page-delimiter "") (forward-char 1))

Of course, this condition should never be true: if `page-delimiter' is
"", functions from page.el will most probably never work correctly
anyway.  What (maybe) should have been tested would be

(eq (match-beginning 0) (match-end 0))

but anyway, since Emacs regex engine does not have a lot of zero-width
assertions, this is not going to happen very often anyway (certainly
never with the default value of `page-delimiter', which explains why the
OP hasn't noticed any problems with his patch).  One possible value of
`page-delimiter' that comes to my mind which could lead to the above
condition holding would be "^$" -- though I can hardly see any practical
use for it.

So my proposal would be to just delete the offending line altogether.
I'd be very surprised if we heard any complaints afterwards.

BTW, the analogous code for moving back one page seems also suspicious
to me.  I'll look into it tomorrow.

> Thanks.

Best,

-- 
Marcin Borkowski
http://octd.wmi.amu.edu.pl/en/Marcin_Borkowski
Faculty of Mathematics and Computer Science
Adam Mickiewicz University