Re: [PATCH v6 5/6] fast-import: add option command

From: Sverre Rabbelier <srabbelier@gmail.com>
Date: Sep 3, 2009, 04:55 UTC
Message-ID: <fabb9a1e0909022155r254c41c6s9ed962313c241e9@mail.gmail.com>
In-Reply-To: <7vskf4px6j.fsf@alter.siamese.dyndns.org>

Heya,

On Thu, Sep 3, 2009 at 04:41, Junio C Hamano<gitster@pobox.com> wrote:

Show 5 quoted lines

> If "option git something-unknown" is given, it is clear that the tool that
> generated the stream assumed that such an option exists in the importer;
> it might appear prudent to abort the operation.
>
> But what about "option hg something"?

I think we should assume that if we see 'option not-us foo' without a preceeding 'feature not-us-option', the frontend does not require us to understand the option (perhaps because they also specify 'option git foo'.

> If that is the sensible thing to do, then we obviously should ignore
> "option hg anything", but at the same time we should ignore "option git
> we-do-not-know-what-it-does".

Perhaps, frontends could then use 'feature git-quiet-option' if it wants to make sure it is supported.

> I think at least the function should be made conditional to die() if it
> was called from parse_argv() but simply ignore unknown if it was called
> from the input stream.

Makes sense, what do the fast-import devs think?

Show 5 quoted lines

>> +static void parse_option(void)
>> +{
>> +     char* option = command_buf.buf + 11;
>
> ERROR: "foo* bar" should be "foo *bar"

Ah, I thought I had fixed all of those, apologies.

> ERROR: do not use C99 // comments
> ERROR: do not use C99 // comments

Will fix in the next version (after we decide on what to do with unknown git options).

-- 
Cheers,

Sverre Rabbelier