Re: [PATCH 12/17] revert: Save command-line options for continuing operation
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Jul 12, 2011, 05:56 UTC
- Message-ID
- <CALkWK0kxnN_vcvaetOFw1Ouj7vj3mQ2DjPWv5v_9_a5UNjFooQ@mail.gmail.com>
- In-Reply-To
- <7vei1wbjji.fsf@alter.siamese.dyndns.org>
Hi Junio,
Junio C Hamano writes:
Show 18 quoted lines
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>> + die(_("Malformed options sheet: %s"), git_path(SEQ_OPTS_FILE));
>> +}
>> +
>> +static void read_populate_opts(struct replay_opts **opts_ptr)
>
> It would make sense to refactor config.c:git_parse_file() to be easier to
> use, rename it to be more descriptive (it should be a function that takes
> a struct that holds these fields
>
> static FILE *config_file;
> static const char *config_file_name;
> static int config_linenr;
> static int config_file_eof;
>
> in addition to the callback "fn" and callback "data", expects the file to
> be in something similar to the .ini format, parses it with the help from
> the callback function), and use it here, without rolling your own.Yes, and I'm working on it. However, it'll probably take me more than a few iterations to get the patch right- I didn't want the options parser to be blocked by that.
Show 6 quoted lines
> Also I get > > builtin/revert.c:682: error: 'read_populate_todo' defined but not used > builtin/revert.c:848: error: 'read_populate_opts' defined but not used > > at this step in the series, which is not quite optimal.
Ah, yes. I was going to ask about this: "revert: Save data for continuing after conflict resolution" introduces read_ and save_ counterparts for todo "revert: Save command-line options for continuing operation" introduces read_ and save_ counterparts for opts The read_ counterparts of both are only used much later in "revert: Introduce --continue to continue the operation"
Now I could introduce the read_ functions in the last patch, but that wouldn't be very elegant -- I want to show people how to read the todo/ opts file in the same patch where I first announce and show people how to write them. Does that make sense? What do you suggest? Something like the GCC "unused"?
Thanks.
-- Ram