bug#20587: 24.1 forward-line docs inconsistent/surprising return value

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Vivek Dasmohapatra]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1513 bytes --]

In GNU Emacs 24.1.1 (x86_64-pc-linux-gnu, GTK+ Version 2.24.10)
  of 2012-08-23 on noise, modified by Debian

In the elisp manual, (forward-line X) is described as returning
the number of lines left to move if it cannot satisfy the caller's
request to move forward X lines.

However, calling (forward-line 1) at the start of a line with
no terminating newline (ie the incomplete final line in a buffer,
if it is incomplete) returns 0.

Calling (forward-line 1) at the _end_ of such a line returns 1.

The docstring for forward line does say the following at the end:

“With positive N, a non-empty line at the end counts as one line
  successfully moved (for the return value).”

I think this is surprising and inconsistent behaviour. It doesn't seem
reasonable to me, especially given that the documentation starts with:

“Move N lines forward (backward if N is negative).
  Precisely, if point is on line I, move to the start of line I + N
  ("start of line" in the logical order).”

In any case, even if forward-line is not altered (I suppose it is
possible that this behaviour has been around for a long time, and
many things may depend on forward-line behaving in this manner),
the Info manual should be updated to match the docstring.

I also think the surprising behaviour is not clearly flagged: It is
part of a single lump of docstring text, and not flagged or highlighted
in any way (a simple extra blank line before its description, or a
"NOTE:" or similar would make it much easier to spot).

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Eli Zaretskii]

> Date: Sat, 16 May 2015 00:23:56 +0100 (BST)
> From: Vivek Dasmohapatra <vivek@etla.org>
> 
> In the elisp manual, (forward-line X) is described as returning
> the number of lines left to move if it cannot satisfy the caller's
> request to move forward X lines.
> 
> However, calling (forward-line 1) at the start of a line with
> no terminating newline (ie the incomplete final line in a buffer,
> if it is incomplete) returns 0.
> 
> Calling (forward-line 1) at the _end_ of such a line returns 1.
> 
> The docstring for forward line does say the following at the end:
> 
> “With positive N, a non-empty line at the end counts as one line
>   successfully moved (for the return value).”
> 
> I think this is surprising and inconsistent behaviour.

Why do you think it's inconsistent?  In the first case, the command
moved; in the second one it didn't.  So the return value looks OK to
me, and I see that the implementation goes to some length to catch the
first case and return zero.  (The doc string indeed is inaccurate.)

> It doesn't seem reasonable to me, especially given that the
> documentation starts with:
> 
> “Move N lines forward (backward if N is negative).
>   Precisely, if point is on line I, move to the start of line I + N
>   ("start of line" in the logical order).”

Again, the documentation could be made more accurate, but let's first
agree on the desirable behavior, okay?

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Vivek Dasmohapatra]

> Why do you think it's inconsistent?  In the first case, the command

It strikes me as inconsistent because of this:

>>   Precisely, if point is on line I, move to the start of line I + N
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
And it is precisely this which it has not been able to do.

Take the example of a case where I wish to move N lines forward
to leave a gap of N lines before I insert some text, and I use
the return value of forward-line to check how far I have moved.

On an incomplete last line with an N of 1, forward-line will return 0,
indicating that it has satisfied my request to move to the start of
line I + 1, when in fact it has not, so now I must check for end-of-buffer
explicitly to see if this case has occurred.

> moved; in the second one it didn't.  So the return value looks OK to
> me, and I see that the implementation goes to some length to catch the
> first case and return zero.  (The doc string indeed is inaccurate.)

It did not do that. It moved to the end of line I, not the start of
line I + 1.

That's the part I find surprising.

It may be that the behaviour is so ancient that it cannot be altered 
without breaking things: I wouldn't push for the behaviour change in
that case. But I think the special case should be more clearly flagged
and stated.

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Eli Zaretskii]

> Date: Sat, 16 May 2015 11:07:03 +0100 (BST)
> From: Vivek Dasmohapatra <vivek@etla.org>
> cc: 20587@debbugs.gnu.org
> 
> > Why do you think it's inconsistent?  In the first case, the command
> 
> It strikes me as inconsistent because of this:
> 
> >>   Precisely, if point is on line I, move to the start of line I + N
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> And it is precisely this which it has not been able to do.

You are again talking about the doc string.  Let's first talk about
what it should return in this case.  If it returns 1, it would mean it
didn't move, and you'd expect it to be at the beginning of the last
line, not at its end.

IOW, the function moved as far as it could, since there's no next line
in this case.

> Take the example of a case where I wish to move N lines forward
> to leave a gap of N lines before I insert some text, and I use
> the return value of forward-line to check how far I have moved.

You can always test eob to see if this is the case.  It's a special
case anyway.

> On an incomplete last line with an N of 1, forward-line will return 0,
> indicating that it has satisfied my request to move to the start of
> line I + 1, when in fact it has not

But it did move.  Returning zero would mean it didn't, or would be
indistinguishable from the case it didn't.

> It did not do that. It moved to the end of line I, not the start of
> line I + 1.
> 
> That's the part I find surprising.

That's the only logical thing to do in that case.

> It may be that the behaviour is so ancient that it cannot be altered 
> without breaking things: I wouldn't push for the behaviour change in
> that case. But I think the special case should be more clearly flagged
> and stated.

If we agree about the behavior, I can change the doc string.  That's
easy enough.

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Eli Zaretskii]

> Date: Sat, 16 May 2015 16:28:42 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 20587@debbugs.gnu.org
> 
> > It may be that the behaviour is so ancient that it cannot be altered 
> > without breaking things: I wouldn't push for the behaviour change in
> > that case. But I think the special case should be more clearly flagged
> > and stated.
> 
> If we agree about the behavior, I can change the doc string.  That's
> easy enough.

Actually, now that I read the doc string in its entirety, it seems to
say all that already:

  Move N lines forward (backward if N is negative).
  Precisely, if point is on line I, move to the start of line I + N
  ("start of line" in the logical order).
  If there isn't room, go as far as possible (no error).
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  Returns the count of lines left to move.  If moving forward,
  that is N - number of lines moved; if backward, N + number moved.
  With positive N, a non-empty line at the end counts as one line
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  successfully moved (for the return value).
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If those 2 sentences don't describe the situation with a buffer that
ends in a line without a newline, then what is missing or unclear
here, in your opinion?

So are you saying that the only documentation that needs to be
clarified is the ELisp manual?

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Vivek Dasmohapatra]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1606 bytes --]

>  With positive N, a non-empty line at the end counts as one line
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>  successfully moved (for the return value).
>  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>
> If those 2 sentences don't describe the situation with a buffer that
> ends in a line without a newline, then what is missing or unclear
> here, in your opinion?

What is unclear or misleading is, imo, the following:

  - The docstring begins by defining moving forward 1 line as
    moving to the start of the next line.

  - It then states that the function will return the number of lines
    left to move (compared to what the user requested)

  - It states that failing to move the requested number of lines is not
    an error.

So far, so good.

However, it then says, as a sparse footnote:

   “With positive N, a non-empty line at the end counts as one line”

So we have a problem of description:

“a non-empty line at the end counts as one line” is being used to mean
“a partial final line counts as a whole line”. I think this is obscure,
and its meaning is only evident in hindsight, having used the function
and discovered that forward-line returns its complete-success value (0)
even when it has not moved forward a line (per its own definition).

I also happen to think the behaviour is not sensible: If I ask to move 
forward 1 (complete) line and this canot be satisfied, the function
should return 1, as there is still 1 line left to move (but I acknowledge
that this is an old function and many things use it, and this may not
be a safe change).

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Eric Hanchrow]

[-- Attachment #1: Type: text/plain, Size: 1153 bytes --]

For what it's worth, my reaction was just the same as Vivek's:

* The behavior of forward-line surprised me;
* The docstring describes that behavior, albeit without IMHO drawing
sufficient attention to its oddness;
* The Elisp reference is clearly wrong.

Like Vivek, I'd hesitate to actually change the function's behavior, out of
fear that some code somewhere depends on it.  But I would like to see the
Elisp reference changed, perhaps like this:

diff --git a/doc/lispref/positions.texi b/doc/lispref/positions.texi
index e7c79d5..dde4739 100644
--- a/doc/lispref/positions.texi
+++ b/doc/lispref/positions.texi
@@ -359,6 +359,9 @@ If @code{forward-line} encounters the beginning or end
of the buffer (or
 of the accessible portion) before finding that many lines, it sets point
 there.  No error is signaled.

+Perhaps counter-intuitively, if the buffer does not end with a
+newline, the return value is the same as if it did.
+
 @code{forward-line} returns the difference between @var{count} and the
 number of lines actually moved.  If you attempt to move down five lines
 from the beginning of a buffer that has only three lines, point stops at

[-- Attachment #2: Type: text/html, Size: 1449 bytes --]

bug#20587: 24.1 forward-line docs inconsistent/surprising return value

[Eli Zaretskii]

> From: Eric Hanchrow <eric.hanchrow@gmail.com>
> Date: Sat, 23 May 2015 03:26:45 +0000
> 
> For what it's worth, my reaction was just the same as Vivek's:
> 
> * The behavior of forward-line surprised me;
> * The docstring describes that behavior, albeit without IMHO drawing sufficient
> attention to its oddness;
> * The Elisp reference is clearly wrong.

I fixed the ELisp manual and tried to clarify in the doc string the
return value in this case.

Thanks.