Heya,
On Sat, Sep 12, 2009 at 21:04, Shawn O. Pearce <spearce@xxxxxxxxxxx> wrote:
> The more I think about this, I may have to agree with Ian, I'm not
> sure option makes much sense.
Perhaps instead we can replace the option series with a few extra
features? That is 'feature git-quiet' (or maybe just 'feature quiet')
and 'feature git-marks' (or just 'feature marks', since its' fairly
generic)?
> But what should happen if "option import-marks=bleh" isn't
> understood by fast-import? Wouldn't the stream be useless anyway,
> because the marks it assumes aren't present? Or worse, "option
> export-marks=bleh" isn't recognized. The stream imports, but any
> marks it was supposed to store for the frontend to reuse later
> are gone.
This was why originally we aborted when we see an unrecognized option,
I agree that due to how the series has evolved this is not such a good
idea anymore.
> @@ -2485,6 +2518,27 @@ static int git_pack_config(const char *k, const char *v, void *cb)
>> static const char fast_import_usage[] =
>> "git fast-import [--date-format=f] [--max-pack-size=n] [--depth=n] [--active-branches=n] [--export-marks=marks.file]";
>>
>> +static void parse_argv(void)
>> +{
>> + unsigned int i;
>> +
>> + for (i = 1; i < global_argc; i++) {
>> + const char *a = global_argv[i];
>> +
>> + if (*a != '-' || !strcmp(a, "--"))
>> + break;
>> +
>> + /* error on unknown options */
>> + parse_one_option(a + 2, 0);
>> + }
>> + if (i != global_argc)
>> + usage(fast_import_usage);
>> +
>> + seen_non_option_command = 1;
>
> So if I pass a single command line option, like --export-marks,
> we die if we see an "option git " inside of the stream? That's not
> what we wanted to do.
Nope, parse_argv isn't called until after we encounter a non-git
option. I think there's a test that makes sure this works too.
> I thought on fast-import list we agreed that the syntax of option was:
Right.
> So what is this parse_nongit_option() for, other than to obfuscate
> the code? Can't we handle all of this in parse_option, have it
> check the VCS tag, and return early there?
I wanted to make it obvious that we ignore non-git options; depending
on whether we want to keep the option part of this series in the first
place I'll either handle it all in parse_option or drop those patches.
--
Cheers,
Sverre Rabbelier
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html