bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Drew Adams]

You essentially did a global replace of `isearch-word' and
`isearch-new-word' with `isearch-regexp-function' and
`isearch-new-regexp-function'.

You provided a `define-obsolete-variable-alias' for `isearch-word',
which is a global variable.  But you did nothing to accommodate
`isearch-new-word'.

This incompatible change breaks existing code that was promised that it
could set `isearch-new-word' in order to affect the subsequent behavior
(of `isearch-word', now renamed to `isearch-regexp-function').

A user or existing code needs to continue to be able to set
`isearch-new-word' OR `isearch-new-regexp-function'.  If
`isearch-new-word' is bound then `isearch-new-regexp-function' should be
set to its value, so that `isearch-regexp-function' will be properly
updated.

(setq ...
      isearch-regexp-function  (if (boundp 'isearch-new-word)
                                   isearch-new-word
                                 isearch-new-regexp-function)
      ...)

In GNU Emacs 25.0.50.1 (i686-pc-mingw32)
 of 2015-11-12
Bzr revision: 6e5186e8a7ccfb9b8fb35b5f4f0371e4f4a68162
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/snapshot/trunk
 --enable-checking=yes,glyphs --enable-check-lisp-object-type
 --without-compress-install --with-wide-int 'CFLAGS=-O0 -ggdb3'
 LDFLAGS=-Lc:/Devel/emacs/lib 'CPPFLAGS=-DGC_MCHECK=1
 -Ic:/Devel/emacs/include''

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 458 bytes --]

On 26 Nov 2015 5:10 pm, "Drew Adams" <drew.adams@oracle.com> wrote:
>
> You essentially did a global replace of `isearch-word' and
> `isearch-new-word' with `isearch-regexp-function' and
> `isearch-new-regexp-function'.
>
> You provided a `define-obsolete-variable-alias' for `isearch-word',
> which is a global variable.  But you did nothing to accommodate
> `isearch-new-word'.

So adding an alias for isearch-new-word should (in principle) fix the
issue?

[-- Attachment #2: Type: text/html, Size: 646 bytes --]

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Drew Adams]

> > You essentially did a global replace of `isearch-word' and
> > `isearch-new-word' with `isearch-regexp-function' and
> > `isearch-new-regexp-function'.
> >
> > You provided a `define-obsolete-variable-alias' for `isearch-word',
> > which is a global variable.  But you did nothing to accommodate
> > `isearch-new-word'.
> 
> So adding an alias for isearch-new-word should (in principle)
> fix the issue?

I'm not sure that would be the right fix.  I suggested doing this
inside the `let':

(setq ...
      isearch-regexp-function  (if (boundp 'isearch-new-word)
                                   isearch-new-word
                                 isearch-new-regexp-function)
      ...)

This variable, unlike `isearch-word', exists only locally, bound
by `let'.  An alias (e.g., `defvaralias') would make it a global
variable, no?

That was why I made the above suggestion - to avoid creating a
global variable for this.  But feel free to fix it however you
think is best.

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 437 bytes --]

On 26 Nov 2015 11:44 pm, "Drew Adams" <drew.adams@oracle.com> wrote:
> This variable, unlike `isearch-word', exists only locally, bound
> by `let'.

Ah. Now I see it.

> An alias (e.g., `defvaralias') would make it a global
> variable, no?

Yes. I'll probably do that anyway. There's obviously code out there calling
setq on this variable. Since this variable is no longer let bound, these
setq will globally define the variable anyway.

[-- Attachment #2: Type: text/html, Size: 623 bytes --]

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Artur Malabarba]

[-- Attachment #1: Type: text/plain, Size: 98 bytes --]

And IMO, it's good to declare dynamic variable. Specially those used to
interact with the system.

[-- Attachment #2: Type: text/html, Size: 120 bytes --]

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Drew Adams]

> > An alias (e.g., `defvaralias') would make it a global
> > variable, no?
> 
> Yes. I'll probably do that anyway. There's obviously code
> out there calling setq on this variable. Since this variable
> is no longer let bound, these setq will globally define the
> variable anyway.

Any such code out there would be used within the scope of
the macro (i.e., in BODY) - that's the point of such code.
It would count on the `let' binding in `with-search-suspended'.

So an alternative to adding a global binding would be to
first bind `isearch-new-word' to `isearch-regexp-function',
just as was done before.  That DTRT.  Any assignment by setq
in BODY would *not* "globally define the variable anyway."

That would let any 3rd-party code that uses `isearch-new-word'
behave exactly the same as before wrt that local variable.
The behavior could be altered negatively or unexpectedly if
we suddenly change the variable to a defvar.

But this is more of a nit.  I think it makes the behavior
cleaner, but admittedly it makes the code messier.  So please
do whatever you think best, as long as the bug gets fixed.
Thx.

bug#22018: 25.0.50; REGRESSION wrt `isearch-new-word'

[Artur Malabarba]

Fix in emacs-25