bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[Michael Heerdegen]

Hi,

The doc says:

> display-buffer-alist is a variable defined in `window.el'.
> [...]
> 
> Alist of conditional actions for `display-buffer'.
> This is a list of elements (CONDITION . ACTION), where:
> 
>  CONDITION is either a regexp matching buffer names, or a function
>   that takes a buffer and returns a boolean.

But the CONDITION is handled very differently by
`display-buffer-assq-regexp':

(defun display-buffer-assq-regexp (buffer-name alist)
  "Retrieve ALIST entry corresponding to BUFFER-NAME."
  (catch 'match
    (dolist (entry alist)
      (let ((key (car entry)))
	(when (or (and (stringp key)
		       (string-match-p key buffer-name))
		  (and (symbolp key) (functionp key)
		       (funcall key buffer-name alist)))
	  (throw 'match (cdr entry)))))))


Obviously, two things are wrong here:

(1) If CONDITION is a function, it currently must be a symbol.  I don't
see why lambda expressions should be forbidden.  The user should not
need to use defun only to be able specify an argument.

(2) If CONDITION is a function, it takes _two_ arguments: a buffer
_name_ (not a buffer) _and_ an alist.  That should be clarified in the
docstring.


Regards,

Michael.
    





In GNU Emacs 24.2.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.4.2)
 of 2012-11-01 on dex, modified by Debian
 (emacs-snapshot package, version 2:20121101-1)
Windowing system distributor `The X.Org Foundation', version 11.0.10707000
System Description:	Debian GNU/Linux testing (wheezy)

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hi,
>
> The doc says:
>
> > display-buffer-alist is a variable defined in `window.el'.
> > [...]
> > 
> > Alist of conditional actions for `display-buffer'.
> > This is a list of elements (CONDITION . ACTION), where:
> > 
> >  CONDITION is either a regexp matching buffer names, or a function
> >   that takes a buffer and returns a boolean.

And please, also add to the docstring that the CONDITIONS are tried one
after another until one is fulfilled, and that all other entries are
ignored, or something like that.  Just for completeness.


Thanks,

Michael.

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[martin rudalics]

 > And please, also add to the docstring that the CONDITIONS are tried one
 > after another until one is fulfilled, and that all other entries are
 > ignored, or something like that.  Just for completeness.

Hmm...  I tried to do that in revision 110885.  Please have a look.

Thanks again, martin

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

> > And please, also add to the docstring that the CONDITIONS are tried one
> > after another until one is fulfilled, and that all other entries are
> > ignored, or something like that.  Just for completeness.
>
> Hmm...  I tried to do that in revision 110885.  Please have a look.

Looks also ok to me, thanks!


Regards,

Michael.

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[martin rudalics]

 > (defun display-buffer-assq-regexp (buffer-name alist)
 >   "Retrieve ALIST entry corresponding to BUFFER-NAME."
 >   (catch 'match
 >     (dolist (entry alist)
 >       (let ((key (car entry)))
 > 	(when (or (and (stringp key)
 > 		       (string-match-p key buffer-name))
 > 		  (and (symbolp key) (functionp key)
 > 		       (funcall key buffer-name alist)))
 > 	  (throw 'match (cdr entry)))))))
 >
 >
 > Obviously, two things are wrong here:
 >
 > (1) If CONDITION is a function, it currently must be a symbol.  I don't
 > see why lambda expressions should be forbidden.  The user should not
 > need to use defun only to be able specify an argument.

I see no problems with removing the (symbolp key) check.

 > (2) If CONDITION is a function, it takes _two_ arguments: a buffer
 > _name_ (not a buffer) _and_ an alist.  That should be clarified in the
 > docstring.

IIUC the correct solution is to not pass ALIST to the function specified
by `key'.

martin

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[martin rudalics]

 > Obviously, two things are wrong here:
 >
 > (1) If CONDITION is a function, it currently must be a symbol.  I don't
 > see why lambda expressions should be forbidden.  The user should not
 > need to use defun only to be able specify an argument.

I lifted this restriction in revision 110885 on the Emacs-24 release
branch.

 > (2) If CONDITION is a function, it takes _two_ arguments: a buffer
 > _name_ (not a buffer) _and_ an alist.  That should be clarified in the
 > docstring.

As a matter of fact, the code called that function with the variable
`display-buffer-alist' as second argument which doesn't make any sense.
I now pass it the ACTION argument of `display-buffer' instead and tried
to document this in the same revision as above.

Thanks, martin

bug#12854: 24.2.50; `display-buffer-alist': conditions are not handled as documented

[Michael Heerdegen]

martin rudalics <rudalics@gmx.at> writes:

> > (1) If CONDITION is a function, it currently must be a symbol.  I don't
> > see why lambda expressions should be forbidden.  The user should not
> > need to use defun only to be able specify an argument.
>
> I lifted this restriction in revision 110885 on the Emacs-24 release
> branch.

Ok, thanks.  My stuff works now as it's supposed to.

> > (2) If CONDITION is a function, it takes _two_ arguments: a buffer
> > _name_ (not a buffer) _and_ an alist.  That should be clarified in the
> > docstring.
>
> As a matter of fact, the code called that function with the variable
> `display-buffer-alist' as second argument which doesn't make any sense.
> I now pass it the ACTION argument of `display-buffer' instead and tried
> to document this in the same revision as above.

Looks ok to me, the code as well as the doc.


Thanks,

Michael.