bug#1352: 23.0.60; isearch-success-function

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

> Sorry for not responding to your last input after closing this bug.
> I've carefully read you message as I saw it, but found no more
> room for improvement.  The final change was based on your original
> report and approved by Stefan.  I think further trying to achieve
> the perfection will make this worse.
> 
> In particular, your latest suggestions assume that Isearch is 
> a library of
> functions with well-defined interfaces.  This is only partially true.
> Rather it is an editor's feature with a set of subfeatures.  
> So it would
> be more practical to use uniform function names that contain 
> the same name
> prefix for all related functions and variables. This will help better
> recognizing the used subfeature in other features (like Dired 
> and Info),
> helping mentally connect any related function names to it.
> 
> In essence, what you suggest is using the prefix `isearch-visible'
> instead of the current `isearch-filter' in the predicate names.
> I see no preference for a property-based `visible' over an 
> action-based
> `filter'.  I think the word `visible' is more confusing since 
> it can be
> misinterpreted as "visible to the user" instead of "visible 
> to isearch".
> The existing name `isearch-range-invisible' has no such 
> problem because
> it tests whether the search hit is visible to the user.  So the name
> `isearch-filter-invisible' connects two features and names together:
> 
> 1. visible - "visible to the user"
> 2. filter  - "visible to isearch"

Thanks for replying. It's a shame that the bug tracker doesn't take such
followup mails into account. It apparently includes spam but omits messages in a
bug thread after the bug is closed. ;-)

Though I disagree in general, I see your argument, especially the possible
confusion about visibility, which I also pointed out. I'm OK with the convention
you chose, as long as we're consistent.

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

Another thing you might think about is the `-p' ending. Shouldn't we follow that
convention for predicate names? 

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.

You might like to think of these predicates as action functions that perform
filtering, but they are not - they have no side effects. They are just
predicates that can be used by an action function to filter. It's the difference
between `delete-if' (filtering action function) and a predicate passed to it.

That distinction is the main point behind the doc strings and names I suggested.

I'm OK with your approach, but you might want to finish off some of these loose
ends (`-p'; name of the Info predicate; mention return values in doc strings).

Thx - Drew