bug#10040: 24.0.91; isearch.el - binding of user options

bug#10040: 24.0.91; isearch.el - binding of user options

[Drew Adams]

1. I do not hold, as Emacs Dev apparently does (at least has in the
past), that code should never bind user options.  Especially user code
and 3rd-party code.  Code is, after all, one way for users to express
their preferences.
 
2. But doing so is supposedly a no-no from your point of view, based on
preaching I've heard from you over the years.
 
3. And yet you do it, so perhaps you would like to consider code where
you do it as bugs.  Assuming that, let me help by pointing out that you
do it in each of the Isearch help commands: isearch-help-for-help,
isearch-describe-bindings, and isearch-describe-key.  The user options
you let-bind are `same-window-names' and `same-window-regexps'.
 
4. Just as you apparently find a "need" to do this here, so
user/3rd-party code can sometimes find it convenient to bind user
options.
 

In GNU Emacs 24.0.91.1 (i386-mingw-nt5.1.2600) of 2011-11-07 on MARVIN
 Windowing system distributor `Microsoft Corp.', version 5.1.2600
 configured using `configure --with-gcc (4.6) --no-opt --cflags
 -I"D:/devel/emacs/libs/libXpm-3.5.8/include"
 -I"D:/devel/emacs/libs/libXpm-3.5.8/src"
 -I"D:/devel/emacs/libs/libpng-dev_1.4.3-1/include"
 -I"D:/devel/emacs/libs/zlib-dev_1.2.5-2/include"
 -I"D:/devel/emacs/libs/giflib-4.1.4-1/include"
 -I"D:/devel/emacs/libs/jpeg-6b-4/include"
 -I"D:/devel/emacs/libs/tiff-3.8.2-1/include"
 -I"D:/devel/emacs/libs/gnutls-2.10.1/include" --ldflags
 -L"D:/devel/emacs/libs/gnutls-2.10.1/lib"'
 

bug#10040: 24.0.91; isearch.el - binding of user options

[Juri Linkov]

> 3. And yet you do it, so perhaps you would like to consider code where
> you do it as bugs.  Assuming that, let me help by pointing out that you
> do it in each of the Isearch help commands: isearch-help-for-help,
> isearch-describe-bindings, and isearch-describe-key.  The user options
> you let-bind are `same-window-names' and `same-window-regexps'.

As discussed recently in bug#3419, they should be replaced with
`inhibit-same-window':

=== modified file 'lisp/isearch.el'
--- lisp/isearch.el	2011-11-15 14:33:18 +0000
+++ lisp/isearch.el	2011-11-15 19:14:07 +0000
@@ -378,7 +378,7 @@ (make-help-screen isearch-help-for-help-
 (defun isearch-help-for-help ()
   "Display Isearch help menu."
   (interactive)
-  (let (same-window-buffer-names same-window-regexps)
+  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     (isearch-help-for-help-internal))
   (isearch-update))
 
@@ -386,7 +386,7 @@ (defun isearch-describe-bindings ()
   "Show a list of all keys defined in Isearch mode, and their definitions.
 This is like `describe-bindings', but displays only Isearch keys."
   (interactive)
-  (let (same-window-buffer-names same-window-regexps)
+  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     (with-help-window "*Help*"
       (with-current-buffer standard-output
 	(princ "Isearch Mode Bindings:\n")
@@ -395,14 +395,14 @@ (defun isearch-describe-bindings ()
 (defun isearch-describe-key ()
   "Display documentation of the function invoked by isearch key."
   (interactive)
-  (let (same-window-buffer-names same-window-regexps)
+  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     (call-interactively 'describe-key))
   (isearch-update))
 
 (defun isearch-describe-mode ()
   "Display documentation of Isearch mode."
   (interactive)
-  (let (same-window-buffer-names same-window-regexps)
+  (let ((display-buffer-overriding-action '(nil (inhibit-same-window . t))))
     (describe-function 'isearch-forward))
   (isearch-update))
 

bug#10040: 24.0.91; isearch.el - binding of user options

[Juri Linkov]

>> 3. And yet you do it, so perhaps you would like to consider code where
>> you do it as bugs.  Assuming that, let me help by pointing out that you
>> do it in each of the Isearch help commands: isearch-help-for-help,
>> isearch-describe-bindings, and isearch-describe-key.  The user options
>> you let-bind are `same-window-names' and `same-window-regexps'.
>
> As discussed recently in bug#3419, they should be replaced with
> `inhibit-same-window':

Another place where user options `same-window-names' and `same-window-regexps'
are let-bound is `bookmark-bmenu-switch-other-window'.  Similarly we could replace
them with `(display-buffer-overriding-action '(nil (inhibit-same-window . t)))'.

But then I don't understand why its implementation should be different from the
functionally equivalent `Buffer-menu-switch-other-window' that doesn't use
`display-buffer-overriding-action'?

bug#10040: 24.0.91; isearch.el - binding of user options

[Stefan Monnier]

>>> 3. And yet you do it, so perhaps you would like to consider code where
>>> you do it as bugs.  Assuming that, let me help by pointing out that you
>>> do it in each of the Isearch help commands: isearch-help-for-help,
>>> isearch-describe-bindings, and isearch-describe-key.  The user options
>>> you let-bind are `same-window-names' and `same-window-regexps'.

Backward compatibility of the new display-buffer machinery is important,
so for Emacs-24.1 we should focus on making it work with "old code".
For Emacs-24.2 and later, we can switch to eliminating uses of obsolete
vars such as same-window-*.


        Stefan

bug#10040: 24.0.91; isearch.el - binding of user options

[Chong Yidong]

Juri Linkov <juri@jurta.org> writes:

>> As discussed recently in bug#3419, they should be replaced with
>> `inhibit-same-window':

Thanks, I committed your patch to trunk.

> Another place where user options `same-window-names' and
> `same-window-regexps' are let-bound is
> `bookmark-bmenu-switch-other-window'.

I committed a fix for this.  Closing the bug.