Josh Steadmon <steadmon@xxxxxxxxxx> writes:
> From: Glen Choo <chooglen@xxxxxxxxxx>
>
> In a subsequent commit, config parsing will become its own library, and
> it's likely that the caller will want flexibility in handling errors
> (instead of being limited to the error handling we have in-tree).
And the in-tree error handling is abstracted out as the
git_config_err_fn() function; in other words, we become the first
client of the library interface, which makes sense.
> @@ -1035,8 +1088,6 @@ static int git_parse_source(struct config_source *cs, config_fn_t fn,
> int comment = 0;
> size_t baselen = 0;
> struct strbuf *var = &cs->var;
> ...
> + /*
> + * FIXME for whatever reason, do_event passes the _previous_ event, so
> + * in order for our callback to receive the error event, we have to call
> + * do_event twice
> + */
> + do_event(cs, CONFIG_EVENT_ERROR, &event_data);
> + do_event(cs, CONFIG_EVENT_ERROR, &event_data);
> + return -1;
> }
This indeed is very curious and needs to be looked into before we
proceed further. How does the current control flow cope with the
behaviour?
> @@ -2322,7 +2342,9 @@ void read_early_config(config_fn_t cb, void *data)
> */
> void read_very_early_config(config_fn_t cb, void *data)
> {
> - struct config_options opts = { 0 };
> + struct config_options opts = {
> + .parse_options = CP_OPTS_INIT(CONFIG_ERROR_DIE),
> + };
>
> opts.respect_includes = 1;
> opts.ignore_repo = 1;
This uses a bit more assignments to various members of opts. to
initialize it, which could have been done with designated
initializer, like the one in read_protected_config() used to do.
> @@ -2760,12 +2784,14 @@ int repo_config_get_pathname(struct repository *repo,
> static void read_protected_config(void)
> {
> struct config_options opts = {
> - .respect_includes = 1,
> - .ignore_repo = 1,
> - .ignore_worktree = 1,
> - .system_gently = 1,
> + .parse_options = CP_OPTS_INIT(CONFIG_ERROR_DIE),
> };
>
> + opts.respect_includes = 1;
> + opts.ignore_repo = 1;
> + opts.ignore_worktree = 1;
> + opts.system_gently = 1;
> +
It is curious why you want to switch to manual assignment, instead
of keeping the designated initializer for this one. I would have
expected the initialization in read_very_early_config() to start
using designated initializer to be consistent, instead.
Thanks.
