Re: [RFC PATCH 0/3] fast-import: add option command

From: Shawn O. Pearce <spearce@spearce.org>
Date: Aug 3, 2009, 00:15 UTC
Message-ID: <20090803001522.GE1033@spearce.org>
In-Reply-To: <1249189570-26576-1-git-send-email-srabbelier@gmail.com>

Sverre Rabbelier <srabbelier@gmail.com> wrote:

Show 5 quoted lines

>     This allows the frontend to specify any of the supported options as
>     long as no non-option command has been given. This way the
>     user does not have to include any frontend-specific options, but
>     instead she can rely on the frontend to tell fast-import what it
>     needs.

...

>  fast-import.c          |  137 ++++++++++++++++++++++++++++++++++++-----------
>  t/t9300-fast-import.sh |   33 ++++++++++++
>  2 files changed, 138 insertions(+), 32 deletions(-)

Some comments:

Since you are changing the language format, please also update the documentation of the language.

It might be cleaner to say "option foo=value\n" because then the if block to parse the command line and the if block to parse the input stream are the same. When parsing argv just skip the "--" and pass the rest of the argv value into the function, when parsing the stream, just skip the "option " and pass the rest of the line into the function.

This has come up before on the list. We had somewhat decided against setting options in the stream header. The only option class that really impacts the data itself is the date format, all others are about local file paths or how noisy the command should be. Thus we decided that the frontend should invoke `git fast-import` itself if it cared about these options, and that's what any typical frontend does.

-- 
Shawn.