[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: |
Sat, 20 Dec 2008 14:33:46 -0800 |
> 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