[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#1352: 23.0.60; isearch-success-function
From: |
Juri Linkov |
Subject: |
bug#1352: 23.0.60; isearch-success-function |
Date: |
Mon, 22 Dec 2008 03:23:04 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.0.60 (x86_64-pc-linux-gnu) |
> 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/