Subject: bug#1352: 23.0.60; isearch-success-function

From: Juri Linkov <[email-masked]>
Date: Mon, 22 Dec 2008 03:23:04 +0200

> 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.

> 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').  But filter predicates
already have a prefix `isearch-filter' that indicates that a function
is a filter.  Adding `-p' will make such names more ugly.

> 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'.

-- 
Juri Linkov
http://www.jurta.org/emacs/