GNU bug report logs - #14714
24.3.50; `isearch-filter-predicate(s)'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 14714 in the body.
You can then email your comments to 14714 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50; `isearch-filter-predicate(s)'
Date: Tue, 25 Jun 2013 10:26:15 -0700 (PDT)

Problem similar to that of bug #14712, regarding obsolescence, but I
think there is a code problem also.

1. `isearch-filter-predicates' should not be an _alias_ for
`isearch-filter-predicate'.  The former cannot just replace the latter.

2. I think one cannot currently use a single predicate as the value of
`isearch-filter-predicates'.  There is one place in the code where that
might work, because of this:

(if (consp isearch-filter-predicates)
    isearch-filter-predicates
  (list isearch-filter-predicates))

(But what if isearch-filter-predicates is nil?  You get `(nil)', which I
think is wrong.)

And in other places it should not work at all.  E.g.:

(run-hook-with-args-until-failure
  'isearch-filter-predicates
  (match-beginning 0) (match-end 0))

To make it work throughout, I think you would need to do something like this:

(when (and isearch-filter-predicates
           (atom isearch-filter-predicates))
  (setq isearch-filter-predicates (list isearch-filter-predicates)))

IOW, at the outset, convert an atomic (and non-nil) value to a cons.






In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-06-20 on ODIEONE
Bzr revision: 113100 eliz <at> gnu.org-20130620173624-w9v620tog4yacftk
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=/c/Devel/emacs/binary --enable-checking=yes,glyphs
 CFLAGS=-O0 -g3 LDFLAGS=-Lc:/Devel/emacs/lib
 CPPFLAGS=-Ic:/Devel/emacs/include'

Message #8 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 14714 <at> debbugs.gnu.org
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Tue, 25 Jun 2013 14:46:49 -0400

> 1. `isearch-filter-predicates' should not be an _alias_ for
> `isearch-filter-predicate'.  The former cannot just replace the latter.

Actually, shouldn't we revert this change and use
(add-function :before-while ...) on isearch-filter-predicate instead?
Admittedly, currently nadvice.el does not give you access to the list of
functions so you can't directly do the equivalent of

		   (mapconcat (lambda (s)
				(and (symbolp s)
				     (get s 'isearch-message-prefix)))
			      (if (consp isearch-filter-predicates)
				  isearch-filter-predicates
				(list isearch-filter-predicates))
			      "")

but that could be remedied (even using an advice's property rather than
a symbol property on the function added as advice).


        Stefan

Message #11 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14714 <at> debbugs.gnu.org
Subject: RE: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Tue, 25 Jun 2013 13:18:23 -0700 (PDT)

> Actually, shouldn't we revert this change and use
> (add-function :before-while ...) on isearch-filter-predicate instead?

Why should we do that?  Why is that preferable to using a list of predicates as we do now?

Message #14 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>, Drew Adams <drew.adams <at> oracle.com>
Cc: 14714 <at> debbugs.gnu.org
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Tue, 25 Jun 2013 20:20:08 -0400

>> Actually, shouldn't we revert this change and use
>> (add-function :before-while ...) on isearch-filter-predicate instead?
> Why should we do that?

Because it's better.

The real question is "why shouldn't we".  Jury?


        Stefan

Message #17 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org
Subject: RE: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Tue, 25 Jun 2013 19:34:46 -0700 (PDT)

> >> Actually, shouldn't we revert this change and use
> >> (add-function :before-while ...) on isearch-filter-predicate instead?
> >
> > Why should we do that?
> 
> Because it's better.

You think it is better, clearly.  No one questioned that.

Can you please say why you think so?  Do you imagine it is obvious?
It is not, to me at least.

Message #20 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Wed, 26 Jun 2013 09:08:59 -0400

Drew Adams wrote:
> Can you please say why you think so?

Please don't hijack this discussion.  Juri, could you answer
my question?


        Stefan

Message #23 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org
Subject: RE: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Wed, 26 Jun 2013 07:19:34 -0700 (PDT)

> > Can you please say why you think so?
> > Do you imagine it is obvious?  It is not, to me at least.
> 
> Please don't hijack this discussion.

I am not hijacking anything.  I am the one who filed this bug report.

And I proposed a simple fix for it from the outset - a minor correction of what Juri had already coded and along the lines of what has been done in the past.

You proposed a different fix.  Fine.  But you gave no reason - nothing about any problems with the more usual approach or any advantages of your new approach.

I asked about that.  I am interested in learning the advantages of your novel approach of (IIUC) advising a single hook function as a substitute for allowing for a sequence of hook functions.

I did not say that your solution is not a good one.  I assume the contrary, in fact.  I am asking to understand why you think it is a better one.  That's all.

Understanding this can help with other code as well, presumably.  Is there a reason why you do not want to share your understanding?

Message #26 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: Juri Linkov <juri <at> jurta.org>, 14714 <at> debbugs.gnu.org
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Wed, 26 Jun 2013 10:36:44 -0400

>> > Can you please say why you think so?
>> > Do you imagine it is obvious?  It is not, to me at least.
>> Please don't hijack this discussion.
> I am not hijacking anything.

You are: the discussion is about problems with the way
isearch-filter-predicate was made obsolete and I suggest to simply fix
it by reverting this "obsoletion".

If you want to discuss the merits/demerits of add-function, do
it elsewhere.


        Stefan

Message #29 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Thu, 27 Jun 2013 00:31:13 +0300

> Actually, shouldn't we revert this change and use
> (add-function :before-while ...) on isearch-filter-predicate instead?

Currently I'm exploring possibilities of using `:before-while'
instead of `run-hook-with-args-until-failure'.

IIUC, it can be used to replace hooks, so that for example,
when someone wants to put an additional function on `find-file',
it's possible to use `add-function' instead of appending
that function to `find-file-hook' (or `find-file-hooks'
that is an obsolete alias of `find-file-hook').

But in case of `isearch-filter-predicate', I don't understand
where and on which function to put additional predicate functions.
`isearch-filter-predicate' is a variable, not a function,
so it can't be used for the PLACE argument of `add-function'.
What I'm trying is:

(defvar isearch-filter-predicate nil)
(add-function :before-while isearch-filter-predicate (lambda (b e) (message "b")))
(add-function :after-while  isearch-filter-predicate (lambda (b e) (message "a")))
(funcall isearch-filter-predicate 1 2)

This fails with the error: (void-function nil)

Message #32 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Wed, 26 Jun 2013 21:32:01 -0400

>> Actually, shouldn't we revert this change and use
>> (add-function :before-while ...) on isearch-filter-predicate instead?

> Currently I'm exploring possibilities of using `:before-while'
> instead of `run-hook-with-args-until-failure'.

> IIUC, it can be used to replace hooks, so that for example,
> when someone wants to put an additional function on `find-file',

No, you're thinking of advice-add.  add-function is different (tho it's
used internally by advice-add).
Basically, replace

    (add-hook 'isearch-filter-predicate #'foo nil t)
with
    (add-function :before-while (local isearch-filter-predicate) #'foo)

> (defvar isearch-filter-predicate nil)

For add-function to work well, you want to change this default value to
be a function, such as (lambda () t).


        Stefan

Message #35 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> jurta.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Fri, 28 Jun 2013 02:39:37 +0300

> Basically, replace
>
>     (add-hook 'isearch-filter-predicate #'foo nil t)
> with
>     (add-function :before-while (local isearch-filter-predicate) #'foo)
>
>> (defvar isearch-filter-predicate nil)
>
> For add-function to work well, you want to change this default value to
> be a function, such as (lambda () t).

I'm trying this, but not sure how to implement toggling like
in `dired-isearch-filenames-toggle':

  (setq isearch-filter-predicates
	(if (memq 'dired-isearch-filter-filenames isearch-filter-predicates)
	    (delq 'dired-isearch-filter-filenames isearch-filter-predicates)
	  (cons 'dired-isearch-filter-filenames isearch-filter-predicates)))

Whereas it's clear how to add and remove an advice function, it's unclear
how to check its existence instead of `memq'.  Is `advice-member-p' intended
to do what `memq' does?  When I tried

  (defvar-local isearch-filter-predicate (lambda (b e) t))
  (defun test-b (b e) (message "b"))
  (defun test-a (b e) (message "a"))
  (add-function :before-while (local isearch-filter-predicate) #'test-b)
  (add-function :after-while  (local isearch-filter-predicate) #'test-a)

then

  (funcall isearch-filter-predicate 1 2)

works fine, but

  (advice-member-p #'test-a 'isearch-filter-predicate)

returns nil.

Message #38 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Wed, 03 Jul 2013 20:26:47 -0400

> works fine, but
>   (advice-member-p #'test-a 'isearch-filter-predicate)
> returns nil.

Indeed, advice-member-p is for advice-add but there's no equivalent for
add-function.  Same kind of problem (tho reverted) for the new
advice-mapc.
Hmm... I think I see how to solve these problems, a patch should follow
"shortishly",


        Stefan

Message #41 received at 14714 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Juri Linkov <juri <at> jurta.org>
Cc: 14714 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Sun, 04 Aug 2013 03:45:34 -0400

>> works fine, but
>> (advice-member-p #'test-a 'isearch-filter-predicate)
>> returns nil.

> Indeed, advice-member-p is for advice-add but there's no equivalent for
> add-function.  Same kind of problem (tho reverted) for the new
> advice-mapc.
> Hmm... I think I see how to solve these problems, a patch should follow
> "shortishly",

You should now be able to use

    (advice-function-member-p #'test-a isearch-filter-predicate)
and
    (advice-function-mapc <somefunc> isearch-filter-predicate)


        Stefan

Message #46 received at 14714-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: 14714-done <at> debbugs.gnu.org
Subject: Re: bug#14714: 24.3.50; `isearch-filter-predicate(s)'
Date: Mon, 05 Aug 2013 14:07:33 -0400

I have now reverted to isearch-filter-predicate.

>> works fine, but
>> (advice-member-p #'test-a 'isearch-filter-predicate)
>> returns nil.

Actually, I changed the code in a way that doesn't require
advice-(function-)member-p.


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.