unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Juri Linkov <juri@jurta.org>
To: "Drew Adams" <drew.adams@oracle.com>
Cc: 1352@emacsbugs.donarmstrong.com
Subject: bug#1352: 23.0.60; isearch-success-function
Date: Mon, 22 Dec 2008 03:23:04 +0200	[thread overview]
Message-ID: <87d4flukgn.fsf@jurta.org> (raw)
In-Reply-To: <000801c962f3$05f00200$0200a8c0@us.oracle.com> (Drew Adams's message of "Sat, 20 Dec 2008 14:33:46 -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.

> 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/






  reply	other threads:[~2008-12-22  1:23 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
2008-12-22  1:23             ` Juri Linkov [this message]
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=87d4flukgn.fsf@jurta.org \
    --to=juri@jurta.org \
    --cc=1352@emacsbugs.donarmstrong.com \
    --cc=drew.adams@oracle.com \
    /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).