bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Drew Adams]

The doc strings say nothing about parameter ALIST, which in many cases
is the most important parameter.

One can guess that it is an alist.  And one can guess, for the doc
strings that mention `inhibit-switch-frame' or
`pop-up-frame-parameters', that one or more of its entries can have such
a key, but that's all.  No guessing helps for
`display-buffer-(below-selected|at-bottom|in-previous-window|
use-some-window|no-window|same-window|assq-regexp)'.  Likewise, some of
the "internal" functions.

Please describe what ALIST is for, and anything else that might be
important about its value, for each function.  Function doc strings
should describe all parameters.  You can refer to the doc of another
function, if that is appropriate.  But you must not leave users hanging,
with no explanation.



In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2014-10-20 on LEG570
Bzr revision: 118168 rgm@gnu.org-20141020195941-icp42t8ttcnud09g
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --enable-checking=yes,glyphs CPPFLAGS=-DGLYPH_DEBUG=1'

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Lars Ingebrigtsen]

Drew Adams <drew.adams@oracle.com> writes:

> The doc strings say nothing about parameter ALIST, which in many cases
> is the most important parameter.
>
> One can guess that it is an alist.  And one can guess, for the doc
> strings that mention `inhibit-switch-frame' or
> `pop-up-frame-parameters', that one or more of its entries can have such
> a key, but that's all.  No guessing helps for
> `display-buffer-(below-selected|at-bottom|in-previous-window|
> use-some-window|no-window|same-window|assq-regexp)'.  Likewise, some of
> the "internal" functions.

It is indeed pretty obscure what ALIST is.  Some of the functions say
things like

---
If ALIST contains a ’window-min-height’ entry, this function
ensures that the window used is or can become at least as high as
specified by that entry’s value. 
---

but looking at the code, it passes ALIST around a lot.  So is ALIST a
general buffer/window property thing?  At least one of these doc strings
should say what it is, and the rest could refer to it.

I've Cc'd Martin.

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

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 >> The doc strings say nothing about parameter ALIST, which in many cases
 >> is the most important parameter.
 >>
 >> One can guess that it is an alist.  And one can guess, for the doc
 >> strings that mention `inhibit-switch-frame' or
 >> `pop-up-frame-parameters', that one or more of its entries can have such
 >> a key, but that's all.  No guessing helps for
 >> `display-buffer-(below-selected|at-bottom|in-previous-window|
 >> use-some-window|no-window|same-window|assq-regexp)'.  Likewise, some of
 >> the "internal" functions.
 >
 > It is indeed pretty obscure what ALIST is.

This problem is with us since day one of that buffer display code.
It's probably due to the fact that the 'display-buffer-*' functions
are inherently internal but never have been marked as such in order to
describe them in the manual and allow users to add new ones.

 > Some of the functions say
 > things like
 >
 > ---
 > If ALIST contains a ’window-min-height’ entry, this function
 > ensures that the window used is or can become at least as high as
 > specified by that entry’s value.
 > ---
 >
 > but looking at the code, it passes ALIST around a lot.  So is ALIST a
 > general buffer/window property thing?

 > At least one of these doc strings
 > should say what it is, and the rest could refer to it.

Saying what it is is non-trivial, see section 28.13.3 of the Elisp
manual.  So I have no good idea how to do what you propose.

martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Sat, 3 Aug 2019 09:58:18 +0200
> Cc: 19461@debbugs.gnu.org
> 
> This problem is with us since day one of that buffer display code.
> It's probably due to the fact that the 'display-buffer-*' functions
> are inherently internal but never have been marked as such in order to
> describe them in the manual and allow users to add new ones.

What are the non-internal functions which should be called instead of
these ones (and will call these ones when needed)?

>  > At least one of these doc strings
>  > should say what it is, and the rest could refer to it.
> 
> Saying what it is is non-trivial, see section 28.13.3 of the Elisp
> manual.  So I have no good idea how to do what you propose.

I gave it a go on the master branch, please take a look.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 >> This problem is with us since day one of that buffer display code.
 >> It's probably due to the fact that the 'display-buffer-*' functions
 >> are inherently internal but never have been marked as such in order to
 >> describe them in the manual and allow users to add new ones.
 >
 > What are the non-internal functions which should be called instead of
 > these ones (and will call these ones when needed)?

There is just one and that is 'display-buffer' itself.  Functions like
'display-buffer-pop-up-window' or 'display-buffer-same-window' should
never get called by anyone but 'display-buffer' or a function
(in-)directly called by 'display-buffer'.  'display-buffer' is the
only function that can handle precedences correctly and knows how to
build ALIST from its various ingredients.  In fact, one can rewrite
any 'display-buffer-*' function with the help of 'display-buffer' and
an appropriate ACTION argument.

 >> Saying what it is is non-trivial, see section 28.13.3 of the Elisp
 >> manual.  So I have no good idea how to do what you propose.
 >
 > I gave it a go on the master branch, please take a look.

Good!  Referring to the Elisp manual seems like the only way to add
something useful.

Thanks, martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> What are the non-internal functions which should be called instead of
>> these ones (and will call these ones when needed)?
>
> There is just one and that is 'display-buffer' itself.

Perhaps the doc strings of the other functions should say so?  Or
perhaps be renamed to --pop-up-window (etc)?

>> I gave it a go on the master branch, please take a look.
>
> Good!  Referring to the Elisp manual seems like the only way to add
> something useful.

Yup; looks good.

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

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  drew.adams@oracle.com,
>   19461@debbugs.gnu.org
> Date: Sat, 03 Aug 2019 13:02:47 +0200
> 
> martin rudalics <rudalics@gmx.at> writes:
> 
> >> What are the non-internal functions which should be called instead of
> >> these ones (and will call these ones when needed)?
> >
> > There is just one and that is 'display-buffer' itself.
> 
> Perhaps the doc strings of the other functions should say so?

I think they should, yes.

> Or perhaps be renamed to --pop-up-window (etc)?

That ship has sailed, so I don't think we should/could rename those
functions.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

> There is just one and that is 'display-buffer' itself.  Functions like
> 'display-buffer-pop-up-window' or 'display-buffer-same-window' should
> never get called by anyone

I think the docstrings must tell that.  Because the names suggest the
opposite.  Something like "this function is for internal use only - it
is valid as an action function in the ALIST argument of
display-buffer...don't call it directly" or so?

Problematic in this regard is that there _are_ other functions like that
that are not internal AFAIK - `pop-to-buffer' for example.  How can one
distinguish them?

Michael.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 > I think the docstrings must tell that.  Because the names suggest the
 > opposite.  Something like "this function is for internal use only

Readers might then ask why it does not use the "--" notation.

 > - it
 > is valid as an action function in the ALIST argument of
 > display-buffer

'display-buffer' calls it ACTION argument.  A problem with this is
that so far we do not specify what a "valid" action function is.

 > ...don't call it directly" or so?

A possible text would be:

"This is an action function for buffer display.  It should be called
only by 'display-buffer' or a function directly or indirectly called
by the latter."

Or with a reference to the Elisp manual:

"This is an action function for buffer display, see Info node `(elisp)
Buffer Display Action Functions'.  It should be called only by
'display-buffer' or a function directly or indirectly called by the
latter."

 > Problematic in this regard is that there _are_ other functions like that
 > that are not internal AFAIK - `pop-to-buffer' for example.  How can one
 > distinguish them?

I see no problems since these would not provide such an explanation.

martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

> 'display-buffer' calls it ACTION argument.  A problem with this is
> that so far we do not specify what a "valid" action function is.

Maybe that should be done.  Would there be more to say than that an
action function should either return a window (success) or nil (fail),
and respect the specified specifications in the ALIST, and what actions
are predefined?

> A possible text would be:
>
> "This is an action function for buffer display.  It should be called
> only by 'display-buffer' or a function directly or indirectly called
> by the latter."
>
> Or with a reference to the Elisp manual:
>
> "This is an action function for buffer display, see Info node `(elisp)
> Buffer Display Action Functions'.  It should be called only by
> 'display-buffer' or a function directly or indirectly called by the
> latter."

Sounds ok.  AFAIU this covers defining new actions by using existing
ones, like in

(defun my-display-buffer-pop-up-frame-dedicated (b alist)
  (let ((w (display-buffer-pop-up-frame b alist)))
    (when (windowp w) (set-window-dedicated-p w t))
    w))

right?

Michael.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> I think the docstrings must tell that.  Because the names suggest the
>> opposite.  Something like "this function is for internal use only
>
> Readers might then ask why it does not use the "--" notation.

And the answer is the same as with all the other internal functions that
don't -- historical reasons.

> "This is an action function for buffer display.  It should be called
> only by 'display-buffer' or a function directly or indirectly called
> by the latter."
>
> Or with a reference to the Elisp manual:
>
> "This is an action function for buffer display, see Info node `(elisp)
> Buffer Display Action Functions'.  It should be called only by
> 'display-buffer' or a function directly or indirectly called by the
> latter."

Sounds good to me.

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

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > Readers might then ask why it does not use the "--" notation.
>
> And the answer is the same as with all the other internal functions
> that don't -- historical reasons.

My opinion is: when calling the function from Lisp it is internal, but
the name is public.  It would look wrong if users would have to write
"--" named functions in the ACTION argument.  So it's better to avoid
the word "internal" as in Martins suggestion.

Michael.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Eli Zaretskii]

> Cc: Eli Zaretskii <eliz@gnu.org>, larsi@gnus.org, 19461@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Sun, 4 Aug 2019 10:01:05 +0200
> 
>  > ...don't call it directly" or so?
> 
> A possible text would be:
> 
> "This is an action function for buffer display.  It should be called
> only by 'display-buffer' or a function directly or indirectly called
> by the latter."
> 
> Or with a reference to the Elisp manual:
> 
> "This is an action function for buffer display, see Info node `(elisp)
> Buffer Display Action Functions'.  It should be called only by
> 'display-buffer' or a function directly or indirectly called by the
> latter."

Please install the 2nd variant, it sounds like the best alternative.

Thanks.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Drew Adams]

> Saying what it is is non-trivial, see section 28.13.3 of the Elisp
> manual.  So I have no good idea how to do what you propose.

If saying what it is is non-trivial for the
maintainers, and I'm sure you're right about
that, then imagine the difficulty of a user to
understand what it is, especially if we do not
say what it is.

If it can be said only in the manual, then the
doc string can, I guess, just punt with a link
to the appropriate section of the manual.

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 >> 'display-buffer' calls it ACTION argument.  A problem with this is
 >> that so far we do not specify what a "valid" action function is.
 >
 > Maybe that should be done.  Would there be more to say than that an
 > action function should either return a window (success) or nil (fail),

I think that's already all that can be said, maybe including the
special value 'fail' for 'display-buffer-no-window'.

 > and respect the specified specifications in the ALIST,

An action function should respect the specifications provided that's
possible.  But due to fact that action lists are combined from various
sources, I'm quite confident that it's possible to construct an
inherently invalid ALIST where an action function "inherits" an ALIST
entry originally meant for another action that failed to produce a
window.  In such case, the display action should do its best to not
respect that entry.

 > and what actions
 > are predefined?

The predefined actions may vary continuously so that's probably not
very feasible.

 >> "This is an action function for buffer display, see Info node `(elisp)
 >> Buffer Display Action Functions'.  It should be called only by
 >> 'display-buffer' or a function directly or indirectly called by the
 >> latter."
 >
 > Sounds ok.  AFAIU this covers defining new actions by using existing
 > ones, like in
 >
 > (defun my-display-buffer-pop-up-frame-dedicated (b alist)
 >    (let ((w (display-buffer-pop-up-frame b alist)))
 >      (when (windowp w) (set-window-dedicated-p w t))
 >      w))
 >
 > right?

You mean in the sense that you do not have to provide that text in the
doc-string of 'my-display-buffer-pop-up-frame-dedicated' too?  I think
so.  But I wonder why you don't provide a non-nil 'dedicated' entry in
ALIST here.

martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 >> A possible text would be:
 >>
 >> "This is an action function for buffer display.  It should be called
 >> only by 'display-buffer' or a function directly or indirectly called
 >> by the latter."
 >>
 >> Or with a reference to the Elisp manual:
 >>
 >> "This is an action function for buffer display, see Info node `(elisp)
 >> Buffer Display Action Functions'.  It should be called only by
 >> 'display-buffer' or a function directly or indirectly called by the
 >> latter."
 >
 > Please install the 2nd variant, it sounds like the best alternative.

Done.

martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 >> Saying what it is is non-trivial, see section 28.13.3 of the Elisp
 >> manual.  So I have no good idea how to do what you propose.
 >
 > If saying what it is is non-trivial for the
 > maintainers, and I'm sure you're right about
 > that, then imagine the difficulty of a user to
 > understand what it is, especially if we do not
 > say what it is.

Non-trivial means that it would require some twenty or more lines of
text to describe the construction of the ALIST argument from its
various sources not even counting a listing of its potential
constituents.  And this would then have to be duplicated among the
doc-strings of all these functions.

 > If it can be said only in the manual, then the
 > doc string can, I guess, just punt with a link
 > to the appropriate section of the manual.

Eli has done that now.

martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Stefan Kangas]

tags 19461 fixed
thanks

martin rudalics <rudalics@gmx.at> writes:

>>> A possible text would be:
>>>
>>> "This is an action function for buffer display.  It should be called
>>> only by 'display-buffer' or a function directly or indirectly called
>>> by the latter."
>>>
>>> Or with a reference to the Elisp manual:
>>>
>>> "This is an action function for buffer display, see Info node `(elisp)
>>> Buffer Display Action Functions'.  It should be called only by
>>> 'display-buffer' or a function directly or indirectly called by the
>>> latter."
>>
>> Please install the 2nd variant, it sounds like the best alternative.
>
> Done.
>
> martin

It seems like this bug report was fixed by installing the above change.
Is there more to do here, or can this bug report be closed?

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[martin rudalics]

 > It seems like this bug report was fixed by installing the above change.
 > Is there more to do here, or can this bug report be closed?

It seems you already closed it.  Nothing more to do here in either case.

Thanks, martin

bug#19461: 25.0.50; doc of `display-buffer-*' functions that take ALIST arg

[Stefan Kangas]

martin rudalics <rudalics@gmx.at> writes:

>  > It seems like this bug report was fixed by installing the above change.
>  > Is there more to do here, or can this bug report be closed?
>
> It seems you already closed it.  Nothing more to do here in either case.

Thanks, I just added the fixed tag, but I'm closing it now.