From: "Drew Adams" <drew.adams@oracle.com>
To: "'Juri Linkov'" <juri@jurta.org>
Cc: 1352@emacsbugs.donarmstrong.com
Subject: bug#1352: 23.0.60; isearch-success-function
Date: Sat, 20 Dec 2008 14:33:46 -0800 [thread overview]
Message-ID: <000801c962f3$05f00200$0200a8c0@us.oracle.com> (raw)
In-Reply-To: <87hc4yttax.fsf@jurta.org>
> 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
next prev parent reply other threads:[~2008-12-20 22:33 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <87tza7kwmt.fsf@jurta.org>
2008-11-15 23:39 ` bug#1352: 23.0.60; isearch-success-function Drew Adams
2008-11-16 1:10 ` Stefan Monnier
2008-11-16 1:22 ` Drew Adams
2008-11-16 21:16 ` Juri Linkov
2008-11-16 23:07 ` Stefan Monnier
2008-11-17 0:45 ` Drew Adams
2008-12-20 21:45 ` Juri Linkov
2008-12-20 22:33 ` Drew Adams [this message]
2008-12-22 1:23 ` Juri Linkov
2008-12-22 1:57 ` Drew Adams
2008-12-22 12:32 ` Stefan Monnier
2008-11-17 0:55 ` bug#1352: marked as done (23.0.60; isearch-success-function) Emacs bug Tracking System
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='000801c962f3$05f00200$0200a8c0@us.oracle.com' \
--to=drew.adams@oracle.com \
--cc=1352@emacsbugs.donarmstrong.com \
--cc=juri@jurta.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).