Re: Some testing issues

[Stephen Berman]

On Fri, 14 Jul 2017 09:44:10 -0400 Stefan Monnier <address@hidden> wrote:

>> Well, as I noted, when pos-visible-in-window-p is called in
>> todo-toggle-view-done-items, the selected window is displaying the
>> current buffer and AFAICT that can't go wrong when using todo-mode as
>> intended (i.e., not invoking todo-toggle-view-done-items outside of
>> todo-mode).
>
> Then I suggest you add a (cl-assert (eq (current-buffer)
> (window-buffer))) and then declare that it's the caller's responsibility
> (e.g. the test environment) to make sure this is true.

The tests have assumed this responsibility with the invocation of
set-window-buffer.

> This said, another approach is to say that `recenter` is simply not
> needed in case the buffer is not displayed anywhere (which could
> presumably happen if you call this code from some ad-hoc Elisp
> function), so if (eq (current-buffer) (window-buffer)) is nil, just
> don't bother checking visibility nor recentering.

If I'm not misunderstanding you, I think adding this check, or the above
assertion, to this defun in todo-mode.el is an unwarrented precaution,
because it assumes a use of todo-toggle-view-done-items other than the
one it was defined for.  In principle that's possible, but the same goes
for any Elisp command, and the vast majority of them surely take no such
precautions.  There are a few todo-mode commands that are intended to be
called from outside of todo-mode, and for these I have tried to make
sure they DTRT, but for the others, as with most specialized elisp
commands, don't you think it's reasonable, and sufficient, to rely on
the user's common sense?  (The case of tests is, of course, different,
but as you note, it is (or should be) their responsibility to satisfy
the assumptions of the code being tested, I think even if the
assumptions aren't made explicit in the code.)

Steve Berman