From: Sverre Rabbelier Date: Fri, 14 Aug 2009 16:37:57 GMT Subject: Re: [PATCH v3 2/3] fast-import: add option command Message-ID: In-Reply-To: <20090814153707.GT1033@spearce.org> Heya, On Fri, Aug 14, 2009 at 08:37, Shawn O. Pearce wrote: > Sverre Rabbelier wrote: >> +`option` >> +~~~~~~~~ >> +Processes the specified option so that git fast-import behaves in a >> +way that suits the frontend's needs. >> +Note that options specified by the frontend are overridden by any >> +options the user may specify to git fast-import itself. > > Wha?  This disagrees with the code. It does? It's saying that the user my override what the frontend specifies, which is what the current version does. > This is a nasty refactoring, I would prefer to see it done in its > own commit, "move option_import_marks so it can be called during > command processing". Ok, will do. >> +     // argv hasn't been parsed yet, do so >> +     if (!seen_non_option_command) >> +             parse_argv(); > > This is too late. No it's not. Earlier in the patch, at the other 'seen_non_option_command', we call parse_argv() as well (which happens when a non-option command is issued). This statement is here to deal with options that affect an empty stream, such as 'git format-patch --import-marks=marks.old --export-marks=marks.new < /dev/null && test_cmp marks.old marks.new'. > So yea, I really do think its a good idea for command line options > to override stream options, despite what Dscho may think.  :-) Which is what the current version does, yes? -- Cheers, Sverre Rabbelier