Re: [PATCH 12/17] revert: Save command-line options for continuing operation

[Ramkumar Ramachandra <artagnon@gmail.com>]

Hi Junio,

Junio C Hamano writes:
> 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.

> 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