[PATCH] rev-list: skip --grep argument in early scan

[PATCH] rev-list: skip --grep argument in early scan

[Azuama Odinaka Maximus]

Signed-off-by: Azuama Odinaka Maximus <azuama69@gmail.com>
---
 builtin/rev-list.c          | 2 ++
 t/t6022-rev-list-missing.sh | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0faa833fac..453d82d25c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -771,6 +771,8 @@ int cmd_rev_list(int argc,
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(repo, arg);
+		} else if (!strcmp(arg, "--grep")) {
+			i++;
 		} else if (!strcmp(arg, "--missing-only")) {
 			arg_missing_only = 1;
 		} else if (!strcmp(arg, "-z")) {
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 1bd2c3bc4f..e87678e468 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -215,6 +215,12 @@ do
 	'
 done
 
+test_expect_success "--grep accepts a pattern beginning with --" '
+	git rev-list --grep=--missing-only --max-count=1 HEAD >expect &&
+	git rev-list --grep --missing-only --max-count=1 HEAD >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success "--missing-only requires --missing=print or --missing=print-info" '
 	test_must_fail git rev-list --missing-only --objects HEAD 2>err &&
 	test_grep "requires --missing=print" err
-- 
2.53.0

Re: [PATCH] rev-list: skip --grep argument in early scan

[Pushkar Singh]

Hi,

Thanks for the patch!

One thing I noticed is that the commit message doesn't have a description.
Could you add a bit of context about the issue you're fixing and why this
change is needed? It'll make it easier for others to understand the reason
behind the patch.

> +               } else if (!strcmp(arg, "--grep")) {
> +                       i++;

Also, could you check if the first scan needs similar handling? With --grep
--exclude-promisor-objects, the first scan might treat
--exclude-promisor-objects
as an option instead of the pattern argument to --grep. A test for this case
would help confirm whether the fix covers both scans.

Thanks!

- Pushkar

[PATCH v2] rev-list: skip --grep argument in early scan

[Azuama Odinaka Maximus]

The early option scan in rev-list does not account for options
that take a separate argument. As a result, a pattern passed to
--grep can be mistaken for another option.

Skip the argument to --grep so that it is not interpreted as an
option during the early scan.
---
 builtin/rev-list.c          |  2 ++
 t/t6022-rev-list-missing.sh | 11 +++++++++++
 2 files changed, 13 insertions(+)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0faa833fac..453d82d25c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -771,6 +771,8 @@ int cmd_rev_list(int argc,
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(repo, arg);
+		} else if (!strcmp(arg, "--grep")) {
+			i++;
 		} else if (!strcmp(arg, "--missing-only")) {
 			arg_missing_only = 1;
 		} else if (!strcmp(arg, "-z")) {
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 1bd2c3bc4f..180e6ac766 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -215,6 +215,17 @@ do
 	'
 done
 
+test_expect_success "--grep accepts a pattern beginning with --" '
+	git rev-list --grep=--missing-only --max-count=1 HEAD >expect &&
+	git rev-list --grep --missing-only --max-count=1 HEAD >actual &&
+	test_cmp expect actual
+'
+test_expect_success "--grep does not trigger --exclude-promisor-objects in early scan" '
+    git rev-list --grep=--exclude-promisor-objects --missing=print HEAD >expect &&
+    git rev-list --grep --exclude-promisor-objects --missing=print HEAD >actual &&
+    test_cmp expect actual
+'
+
 test_expect_success "--missing-only requires --missing=print or --missing=print-info" '
 	test_must_fail git rev-list --missing-only --objects HEAD 2>err &&
 	test_grep "requires --missing=print" err
-- 
2.53.0

Re: [PATCH v2] rev-list: skip --grep argument in early scan

[Pushkar Singh]

Hi Azuma,

>
> Skip the argument to --grep so that it is not interpreted as an
> option during the early scan.
> ---

Can you add a "Changes in v2" section after the --- here, summarizing
what changed from the previous version?
Also, I noticed the "Signed-off-by" line is missing in v2. Could you
add it back?

Other than that, it looks good to me.

- Pushkar

Re: [PATCH v2] rev-list: skip --grep argument in early scan

[Junio C Hamano]

Azuama Odinaka Maximus <azuama69@gmail.com> writes:

> +test_expect_success "--grep accepts a pattern beginning with --" '
> +	git rev-list --grep=--missing-only --max-count=1 HEAD >expect &&
> +	git rev-list --grep --missing-only --max-count=1 HEAD >actual &&
> +	test_cmp expect actual
> +'
> +test_expect_success "--grep does not trigger --exclude-promisor-objects in early scan" '
> +    git rev-list --grep=--exclude-promisor-objects --missing=print HEAD >expect &&
> +    git rev-list --grep --exclude-promisor-objects --missing=print HEAD >actual &&
> +    test_cmp expect actual
> +'

Have a blank line between tests.

Do these tests truly verify anything beyond "the command line parser
is not broken by my commit"?  There is nothing in the history that
would trigger '--missing-only'; there is no positive indication that
the pattern starting with "--" was used by '--grep' at all.  The only
clue that '--missing-only' was not taken as a command line option is
that the command did not fail due to a lack of '--missing=<how>'.

The same issue exists in the second added test.

Thanks.

[PATCH v3] rev-list: skip --grep argument in early scan

[Azuama Odinaka Maximus]

The early option scan in rev-list does not account for options
that take a separate argument. As a result, a pattern passed to
--grep can be mistaken for another option.

Skip the argument to --grep so that it is not interpreted as an
option during the early scan.

The tests check that patterns beginning with "--" are actually
matched by --grep, including when the pattern resembles an option
that the early scan handles specially.

Signed-off-by: Azuama Odinaka Maximus <azuama69@gmail.com>
---
Changes in v3:
- Improve both regression tests to verify that --grep matches the
  expected commit when its pattern begins with "--".
- Add a blank line between the tests.

 builtin/rev-list.c          |  2 ++
 t/t6022-rev-list-missing.sh | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0faa833fac..453d82d25c 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -771,6 +771,8 @@ int cmd_rev_list(int argc,
 			revs.exclude_promisor_objects = 1;
 		} else if (skip_prefix(arg, "--missing=", &arg)) {
 			parse_missing_action_value(repo, arg);
+		} else if (!strcmp(arg, "--grep")) {
+			i++;
 		} else if (!strcmp(arg, "--missing-only")) {
 			arg_missing_only = 1;
 		} else if (!strcmp(arg, "-z")) {
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 1bd2c3bc4f..3757bde708 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -215,6 +215,26 @@ do
 	'
 done
 
+test_expect_success "--grep accepts a pattern beginning with --" '
+        test_commit --no-tag --missing-only &&
+        commit=$(git rev-parse HEAD) &&
+        echo "$commit" >expect &&
+        git rev-list --grep=--missing-only HEAD >actual &&
+        test_cmp expect actual &&
+        git rev-list --grep --missing-only HEAD >actual &&
+        test_cmp expect actual
+'
+
+test_expect_success "--grep does not trigger --exclude-promisor-objects in early scan" '
+        test_commit --no-tag --exclude-promisor-objects &&
+        commit=$(git rev-parse HEAD) &&
+        echo "$commit" >expect &&
+        git rev-list --grep=--exclude-promisor-objects --missing=print HEAD >actual &&
+        test_cmp expect actual &&
+        git rev-list --grep --exclude-promisor-objects --missing=print HEAD >actual &&
+        test_cmp expect actual
+'
+
 test_expect_success "--missing-only requires --missing=print or --missing=print-info" '
 	test_must_fail git rev-list --missing-only --objects HEAD 2>err &&
 	test_grep "requires --missing=print" err
-- 
2.53.0

Re: [PATCH v3] rev-list: skip --grep argument in early scan

[Tanga Joris]

Hello Azuama,

Thanks for working on this.

I applied v3 on top of master (0ffa3e35c7) with "git am -3", because
builtin/rev-list.c has changed since you sent it, so running "git am"
fails. Rebasing on master would help. I built it with NO_RUST=1.
t6022 passes, and both new tests fail when I revert only the change
in builtin/rev-list.c, so they do catch the problem.

On Mon, 26 Oct 2026 at 11:30, Azuama Odinaka Maximus <azuama69@gmail.com> wrote:
> +test_expect_success "--grep accepts a pattern beginning with --" '
> +        test_commit --no-tag --missing-only &&
> +        commit=$(git rev-parse HEAD) &&

These lines are indented with spaces instead of a tab. "git diff
--check" reports "indent with spaces" for all the new test lines.

> +             } else if (!strcmp(arg, "--grep")) {
> +                     i++;

This fixes --grep, but other options that take a separate value
still have the same problem. In a repository whose only commit has
"--missing-only" as both its subject and its author name, with this
patch:

  $ git rev-list --grep --missing-only HEAD
  <the commit>
  $ git rev-list --author --missing-only HEAD
  fatal: --missing-only requires --missing=print or --missing=print-info

--committer and --grep-reflog fail the same way. In revision.c, 19
options are parsed with parse_long_opt(), which accepts the value as
a separate argument (--author, --committer, --since, --until, --skip,
--exclude and others). Should the early scan skip the value of all
of them, or is there a reason to handle only --grep? If this only fixes
--grep for now, the commit message could say so, because its first
paragraph describes the general problem.

Also, the NEEDSWORK comment just above this loop uses "--grep
--exclude-promisor-objects" as its example of what the loop gets
wrong. With this patch that example works, so the comment needs a
new example (--author would do), or it simply becomes misleading.

Thanks,
Tanga

[PATCH v4] rev-list: handle arguments in early option scan

[Azuama Odinaka Maximus]

The early option scan in cmd_rev_list() looks for options that
affect missing-object handling before setup_revisions() parses the
command line. It must not interpret an option's separate argument
as another option.

Use parse_long_opt() to recognize revision options that take
separate arguments, and skip those arguments. Handle -n and
--default as well. Stop scanning at --, where subsequent arguments
are pathspecs.

Add tests to ensure that --grep and --author arguments beginning
with -- are not mistaken for options during the early scan.

Signed-off-by: Azuama Odinaka Maximus <azuama69@gmail.com>
---
 builtin/rev-list.c          | 55 +++++++++++++++++++++++++++++++++++--
 t/t6022-rev-list-missing.sh | 26 ++++++++++++++++++
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 071a0e3cb5..4dd646050a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -744,6 +744,47 @@ static void prepare_maximal_independent(struct rev_info *revs)
 	revs->no_walk = 1;
 }
 
+static int rev_list_option_takes_argument(const char **argv)
+{
+	static const char * const options[] = {
+		"max-count",
+		"max-count-oldest",
+		"skip",
+		"max-age",
+		"since",
+		"since-as-filter",
+		"after",
+		"min-age",
+		"before",
+		"until",
+		"date",
+		"author",
+		"committer",
+		"grep-reflog",
+		"grep",
+		"encoding",
+		"glob",
+		"exclude",
+		"exclude-hidden",
+	};
+	const char *optarg;
+	size_t i;
+
+	if (!argv[1])
+		return 0;
+
+	if (!strcmp(argv[0], "-n") ||
+	    !strcmp(argv[0], "--default"))
+		return 1;
+
+	for (i = 0; i < ARRAY_SIZE(options); i++) {
+		if (parse_long_opt(options[i], argv, &optarg) == 2)
+			return 1;
+	}
+
+	return 0;
+}
+
 int cmd_rev_list(int argc,
 		 const char **argv,
 		 const char *prefix,
@@ -789,9 +830,8 @@ int cmd_rev_list(int argc,
 	 * NEEDSWORK: The next loop is utterly broken.  It tries to
 	 * notice an option is used, but without understanding if each
 	 * option takes an argument, which fundamentally would not
-	 * work.  It would not know "--grep
-	 * --exclude-promisor-objects" is not triggering
-	 * "--exclude-promisor-objects" option, for example.
+	 * work.  It would not know that "--author --missing-only" is not
+	 * triggering "--missing-only" option, for example.
 	 *
 	 * We really need setup_revisions() to have a mechanism to
 	 * allow and disallow some sets of options for different
@@ -802,6 +842,15 @@ int cmd_rev_list(int argc,
 	 */
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
+
+		if (!strcmp(arg, "--"))
+			break;
+
+		if (rev_list_option_takes_argument(&argv[i])) {
+			i++;
+			continue;
+		}
+
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
 			repo->fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 1bd2c3bc4f..9f995968fd 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -215,6 +215,32 @@ do
 	'
 done
 
+test_expect_success "--grep accepts a pattern beginning with --" '
+	test_commit --no-tag --missing-only &&
+	commit=$(git rev-parse HEAD) &&
+	echo "$commit" >expect &&
+	git rev-list --grep=--missing-only HEAD >actual &&
+	test_cmp expect actual &&
+	git rev-list --grep --missing-only HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "--grep does not trigger --exclude-promisor-objects in early scan" '
+	test_commit --no-tag --exclude-promisor-objects &&
+	commit=$(git rev-parse HEAD) &&
+	echo "$commit" >expect &&
+	git rev-list --grep=--exclude-promisor-objects --missing=print HEAD >actual &&
+	test_cmp expect actual &&
+	git rev-list --grep --exclude-promisor-objects --missing=print HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "--author argument is not treated as an option in early scan" '
+	test_commit --no-tag author-test &&
+	git rev-list --author --missing-only HEAD >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success "--missing-only requires --missing=print or --missing=print-info" '
 	test_must_fail git rev-list --missing-only --objects HEAD 2>err &&
 	test_grep "requires --missing=print" err
-- 
2.56.0.386.g979eb7e29a

Re: [PATCH v4] rev-list: handle arguments in early option scan

[Junio C Hamano]

Azuama Odinaka Maximus <azuama69@gmail.com> writes:

> The early option scan in cmd_rev_list() looks for options that
> affect missing-object handling before setup_revisions() parses the
> command line. It must not interpret an option's separate argument
> as another option.
>
> Use parse_long_opt() to recognize revision options that take
> separate arguments, and skip those arguments. Handle -n and
> --default as well. Stop scanning at --, where subsequent arguments
> are pathspecs.
>
> Add tests to ensure that --grep and --author arguments beginning
> with -- are not mistaken for options during the early scan.

The commit log message explains the motivation well, and the attempt
to address the feedback from the previous round by handling all 19
options from revision.c is appreciated.

But the approach of hardcoding an array of option strings that take
arguments is inherently fragile and scales poorly.  By duplicating
parser knowledge into the early scan, this patch falls into the
exact trap described by the NEEDSWORK comment just below it.

> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 071a0e3cb5..4dd646050a 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -744,6 +744,47 @@ static void prepare_maximal_independent(struct rev_info *revs)
>  	revs->no_walk = 1;
>  }
>  
> +static int rev_list_option_takes_argument(const char **argv)
> +{
> +	static const char * const options[] = {
> +		"max-count",
> +		"max-count-oldest",
> +		"skip",
> +		"max-age",
> +		"since",
> +		"since-as-filter",
> +		"after",
> +		"min-age",
> +		"before",
> +		"until",
> +		"date",
> +		"author",
> +		"committer",
> +		"grep-reflog",
> +		"grep",
> +		"encoding",
> +		"glob",
> +		"exclude",
> +		"exclude-hidden",
> +	};

Is this hardcoded list complete?  It misses options parsed via
diff_merges_parse_opts() in diff-merges.c (e.g., --diff-merges), as
well as options handled in diff.c (e.g., --src-prefix and
--dst-prefix), which also take separate arguments and are accepted
by git rev-list.  Would

    $ git rev-list --src-prefix --missing-only HEAD

take --missing-only as an extra option or a prefix?

Thanks.

Re: [PATCH v4] rev-list: handle arguments in early option scan

[Azuama Odinaka Maximus]

I've investigated the relevant parsing paths further:

1. Revision options are handled by handle_revision_opt() in
revision.c. It already determines argument consumption through helpers
such as parse_long_opt(), which returns whether an option uses an
attached or separate argument. However, the function also applies the
option's effects to struct rev_info, so calling it again purely to
determine argument consumption would be unsafe.

2. Revision pseudo-options are handled separately by
handle_revision_pseudo_opt().

3. Diff-merge options are handled by diff_merges_parse_opts() in diff-merges.c.

4. Diff options are defined through add_diff_options() in diff.c,
which provides an existing struct option table. This includes options
such as --src-prefix and --dst-prefix.

This suggests that the early scan needs to obtain argument-consumption
information from the same option definitions or recognition logic used
by normal parsing, rather than maintaining another list of option
names.

One direction I'm considering is separating option recognition and
argument consumption from option application, so that the early scan
and normal parser can share the recognition logic without executing
parsing side effects twice.

I have not settled on the implementation yet, particularly how best to
accommodate the hand-written revision and diff-merge parsers alongside
the diff option table.

Would you consider this a reasonable direction? Or do you see an
existing Git parser interface or abstraction that would be preferable
to extend for this purpose?

Thanks again for the guidance.

On Fri, 6 Nov 2026 at 01:36, Junio C Hamano <gitster@pobox.com> wrote:
>
> Azuama Odinaka Maximus <azuama69@gmail.com> writes:
>
> > The early option scan in cmd_rev_list() looks for options that
> > affect missing-object handling before setup_revisions() parses the
> > command line. It must not interpret an option's separate argument
> > as another option.
> >
> > Use parse_long_opt() to recognize revision options that take
> > separate arguments, and skip those arguments. Handle -n and
> > --default as well. Stop scanning at --, where subsequent arguments
> > are pathspecs.
> >
> > Add tests to ensure that --grep and --author arguments beginning
> > with -- are not mistaken for options during the early scan.
>
> The commit log message explains the motivation well, and the attempt
> to address the feedback from the previous round by handling all 19
> options from revision.c is appreciated.
>
> But the approach of hardcoding an array of option strings that take
> arguments is inherently fragile and scales poorly.  By duplicating
> parser knowledge into the early scan, this patch falls into the
> exact trap described by the NEEDSWORK comment just below it.
>
> > diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> > index 071a0e3cb5..4dd646050a 100644
> > --- a/builtin/rev-list.c
> > +++ b/builtin/rev-list.c
> > @@ -744,6 +744,47 @@ static void prepare_maximal_independent(struct rev_info *revs)
> >       revs->no_walk = 1;
> >  }
> >
> > +static int rev_list_option_takes_argument(const char **argv)
> > +{
> > +     static const char * const options[] = {
> > +             "max-count",
> > +             "max-count-oldest",
> > +             "skip",
> > +             "max-age",
> > +             "since",
> > +             "since-as-filter",
> > +             "after",
> > +             "min-age",
> > +             "before",
> > +             "until",
> > +             "date",
> > +             "author",
> > +             "committer",
> > +             "grep-reflog",
> > +             "grep",
> > +             "encoding",
> > +             "glob",
> > +             "exclude",
> > +             "exclude-hidden",
> > +     };
>
> Is this hardcoded list complete?  It misses options parsed via
> diff_merges_parse_opts() in diff-merges.c (e.g., --diff-merges), as
> well as options handled in diff.c (e.g., --src-prefix and
> --dst-prefix), which also take separate arguments and are accepted
> by git rev-list.  Would
>
>     $ git rev-list --src-prefix --missing-only HEAD
>
> take --missing-only as an extra option or a prefix?
>
> Thanks.

Re: [PATCH v4] rev-list: handle arguments in early option scan

[Junio C Hamano]

Azuama Odinaka Maximus <azuama69@gmail.com> writes:

> I've investigated the relevant parsing paths further:
>
> 1. Revision options are handled by handle_revision_opt() in
> revision.c. It already determines argument consumption through helpers
> such as parse_long_opt(), which returns whether an option uses an
> attached or separate argument. However, the function also applies the
> option's effects to struct rev_info, so calling it again purely to
> determine argument consumption would be unsafe.

Correct.  There is no dry-run mode for this.

> 2. Revision pseudo-options are handled separately by
> handle_revision_pseudo_opt().

Yes, this is called from various code paths.  No dry-run, either.

> 3. Diff-merge options are handled by diff_merges_parse_opts() in diff-merges.c.
>
> 4. Diff options are defined through add_diff_options() in diff.c,
> which provides an existing struct option table. This includes options
> such as --src-prefix and --dst-prefix.

Correct.  I do not think there is any dry-run to any of these.

I am not sure if the above is exhausitve, though.  I didn't check.

> This suggests that the early scan needs to obtain argument-consumption
> information from the same option definitions or recognition logic used
> by normal parsing, rather than maintaining another list of option
> names.

It is what the NEEDSWORK comment boils down to, I think.  I do not
see how the above survey helps with that conclusion, though.

> One direction I'm considering is separating option recognition and
> argument consumption from option application, so that the early scan
> and normal parser can share the recognition logic without executing
> parsing side effects twice.
>
> I have not settled on the implementation yet, particularly how best to
> accommodate the hand-written revision and diff-merge parsers alongside
> the diff option table.
>
> Would you consider this a reasonable direction? Or do you see an
> existing Git parser interface or abstraction that would be preferable
> to extend for this purpose?

All of the above assumes that we want to keep a separate "early
scan", but I wonder if that is really a good direction to go.  If
you can clearly identify the problem the "early scan" wanted to
solve, and can come up with a way to solve that problem without
scanning the list of arguments twice (i.e., not involving an "early
scan" at all), would that give you a cleaner implementation?

If the default of an option (is it '.fetch_if_missing' alone, or are
there many other settings involved?) needs to depend on other
options, perhaps fixing it is simpler if the presence of an explicit
option that sets '.fetch_if_missing' is recorded in a separate
member of the struct, and then at the very end after all the command
line options have been seen, take these other options and the
explicit option setting for it into account to synthesize the
result?  If '.fetch_if_missing' was explicitly set from what the
user said from the command line, there is no need to look at any
other options, and if not, then the settings left by parsing other
options that the misdesigned "early scan" pays attention to can be
consulted to decide what the desirable value should be.

Just thinking out loud.  It seems that this started at df11e19648
(rev-list: support termination at promisor objects, 2017-12-08), and
then other needs may have been piled on top, but going back to that
original commit may be a good place to start understanding and
reevaluating if the "early scan" was the best approach to solve
whatever problem it was trying to solve.

> Thanks again for the guidance.
>
> On Fri, 6 Nov 2026 at 01:36, Junio C Hamano <gitster@pobox.com> wrote:

Never top-post.