bug#24104: 25.1.50; frame-or-buffer-changed-p also check file size

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

> > Assume a buffer BUF, with size, SIZE, returning non-nil for predicate
> > 'buffer-modified-p'; then we write some text on BUF, so that the
> > new size is SIZE_2 != SIZE.
> > In this scenario, 'frame-or-buffer-changed-p' return nil, i.e.,
> > the buffer state appears to not have changed.
> 
> Because it didn't.  The buffer was changed, and it still is.  In
> particular, the original change that caused buffer-modified-p to
> return non-nil could also be (and normally is) a change in the size of
> the buffer.

If the meaning of `frame-or-buffer-changed-p' in this scenario is,
as you suggest, intended to reflect whether or not `buffer-modified-p'
has changed (and not whether the buffer has changed), then its doc
should say so.

Whether `buffer-modified-p' has changed is something different from
whether the buffer state has changed.  If `buffer-modified-p' is
already non-nil then additional changes to the buffer will not cause
a change in `buffer-modified-p', as Tino's example illustrates.

The name `frame-or-buffer-changed-p', and its current doc, suggest
that it should return non-nil if the buffer appears to have changed.
The buffer has changed in this case, even if the value of
`buffer-modified-p' has not changed (it is still non-nil).  But
apparently the buffer does not "appear" to have changed.  That's OK,
but what's involved in this appearing is unclear.

Something should be said in the doc about what kinds of changes lead
to a change "appearing" to have occurred (or what kinds do not lead
to that).  IOW, the doc should say something about how the
"appearance" of change is judged/detected.  If read-only and
modified flags determine this (alone), then the doc should say so.

Something should be said about just what is meant by (included
in) a buffer state change and a frame state change.

If all that is meant, for buffer change, is what is said (later) in
the doc for VARIABLE, i.e., all that is checked are the (1) read-only
and (2) modified flags, then just say that up front, and not only
for VARIABLE.  That, I think, would make most of the above clear.

Except for frame state change - that part is still completely
unspecified, AFAICT.  Just what is meant by (included in) the frame
state, and how is a change in frame state detected/recorded by Emacs
("appear" to have occurred).  For buffer changes: read-only and
modified flags - OK.  What about for frame-state changes?

Finally, the doc should not talk about some "internal vector" BEFORE
that has been introduced.  Assuming that the "an internal variable"
of the last paragraph is what holds "the internal vector", it needs
to be introduced before we talk about "the internal vector".  And
the fact that the value of the internal variable is "the internal
vector" should be made explicit.

The last sentence of the doc is confusing/scary.  If a user can
call the function without passing an argument, is that different
from calling it and passing a nil argument?  If the behavior is
different then this is REALLY an exception - I've never heard of
such a thing.  What is this all about?  I'm guessing that that
last sentence should just be removed.

On the other hand, if the first part of the doc is to be believed,
VARIABLE, if present, must be a symbol that is a variable - which
excludes the symbol `nil'.  In that case it's still not clear to
me how the absence of VARIABLE is distinguished from the presence
of nil as an argument.  Perhaps that is done at the C level and
this is a real exception to the rule of &optional arguments.  If
so, that needs to be spelled out clearly.

If this is really about passing a symbol, and not its value (see
below), then, again, there is no need for the confusing last doc
sentence.  And in that case if a user _does_ pass nil as the
argument then is an error raised saying that nil is a constant and
not a variable?  (And again, how is this case distinguished from
an absence of VARIABLE?)

The doc seems to be essentially backward, and it doesn't make
explicit the connections that I think the author intended.

It should say (IIUC, but I probably do not) that the state of
frames and buffers - or of frame and buffer changes (?) is kept
in a vector that is the value of an internal variable, and which
for buffers records the read-only and modified flags (only), and
which for frames records [???].  The function returns non-nil if
this recorded state indicates that a change has occurred.  It
should then say that you can provide a different such variable
to use by passing it as optional argument VARIABLE.

The doc says that VARIABLE "is a variable name whose value is
...".  The value of a _name_ cannot be nil or a vector.
Presumably what is meant is that VARIABLE is a variable
(symbol) whose value is....  The value of the symbol's _name_
is a string (`symbol-name').

So you pass a symbol and not its value (nil or a vector), I guess.
Why is that?

In sum, this doc needs some work, I think, for it to help users
more and not confuse them more.  It confuses me, at least.

You can't make good use of a function if you don't know what its
intended behavior is.  Of course you can try to discover that by
trial and error ... but that's what we have doc for.