[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#1352: 23.0.60; isearch-success-function
From: |
Drew Adams |
Subject: |
bug#1352: 23.0.60; isearch-success-function |
Date: |
Sun, 21 Dec 2008 17:57:41 -0800 |
> > 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