bug#1352: 23.0.60; isearch-success-function

["Drew Adams" <drew.adams@oracle.com>]

> > Minor point: `Info-isearch-filter-predicate' does not 
> > respect your naming convention: "predicate" does not
> > describe what the filter does. Using your
> > convention, you might call it instead 
> > `Info-isearch-filter-body-text' or
> > `Info-isearch-filter-visible-body-text' (which admittedly 
> > is a bit long).
> 
> I agree that the current name is inappropriate.  But your suggestions
> are no better.  First, there is no such a term as `body' in the Info
> documentation, so it will cause questions what is a body 
> (answer: no header and not tag node).  What is worse is that after
> adding more conditions to this predicate we'll need to rename it.
> I remember long ago you proposed to filter out header lines.  Adding
> such a condition for the header line would require renaming from a
> name like `Info-isearch-filter-node-text' (if this function existed
> at that time) to what you just proposed. In future when it makes
> sense to add more conditions like skipping `* Menu'
> where actually the tag `* Menu' is part of the node's body, the name
> you proposed becomes wrong and needs to be renamed to a name like
> `Info-isearch-filter-body-text-without-menu-tag', and so on.
> 
> That's why I think we should find a general name for the 
> default predicate. I think it should be simply `Info-isearch-filter'
> without any specifics.

Agreed.

> > Another thing you might think about is the `-p' ending. Shouldn't we
> > follow that convention for predicate names?
> 
> `-p' is usually added to short names that have no other indication
> that they are predicates (e.g. `display-images-p').

I don't think that qualification is part of the convention.

> But filter predicates already have a prefix `isearch-filter'
> that indicates that a function is a filter.

As I mentioned, in English, "filter" is ambiguous in this context. It can, as
you intend it, mean a filter (noun), which could be interpreted as a predicate.
(As I pointed out, though, even then it the name doesn't say what the predicate
quality is - what's being filtered in or out.)

But "filter" here can also mean the action (verb) to filter. These functions do
not do any filtering on their own - they have no side effects.

I think the suffix `-p' would help make clear that these are pure predicates,
nothing more. And I think that's the Emacs convention (without your added
qualification), but I could be mistaken about that.

> Adding `-p' will make such names more ugly.

Yes, but also clearer.

> > Especially since the doc strings do not mention the return values.
> > I think a predicate's doc string should state when it returns nil vs
> > non-nil, but if you don't want to do that, then the name 
> > (`-p') would at least give a clue to the type.
> 
> I like your doc strings, so instead of adding `-p' I'll fix 
> doc strings using your variants :-)
> 
> BTW, I noticed that the name `isearch-filter-invisible' is logically
> incorrect because the name should say whether the test is passed.
> So I'll replace it with `isearch-filter-visible'.

That's part of what I was saying in previous feedback I gave. The function name
should, if possible, indicate whether it is filtering out or filtering in (e.g.
keep-lines vs flush-lines).

Anyway, thanks for looking at this again. - Drew