Re: [PATCH v6 3/4] advice: revamp advise API

[Junio C Hamano <gitster@pobox.com>]

"Heba Waly via GitGitGadget" <gitgitgadget@gmail.com> writes:

> diff --git a/advice.c b/advice.c
> index 258cc9ba7af..8d9f2910663 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -32,6 +32,40 @@ int advice_checkout_ambiguous_remote_branch_name = 1;
>  int advice_nested_tag = 1;
>  int advice_submodule_alternate_error_strategy_die = 1;
>  
> +const char ADVICE_ADD_EMBEDDED_REPO[] = "advice.addEmbeddedRepo";
> +const char ADVICE_AM_WORK_DIR[] = "advice.amWorkDir";
> ...
> +static const char *advice_config_keys[] = {
> +	ADVICE_ADD_EMBEDDED_REPO,
> +	ADVICE_AM_WORK_DIR,
> ...
> +/*
> + * To add a new advice, you need to:
> + * Define a new const char array.
> + * Add a new entry to advice_config_keys list.
> + * Add the new config variable to Documentation/config/advice.txt.
> + * Call advise_if_enabled to print your advice.
> + */
> +extern const char ADVICE_ADD_EMBEDDED_REPO[];
> +extern const char ADVICE_AM_WORK_DIR[];
> ...

Hmph.

Even though I said that I would prefer it over the current one,
in that it allows the compilers to catch typo, and over the one
in v5 which uses enum, in that we do not have to go through
enum->string->hash conversion all the time, I have to say that I
am not very happy that we'd need to make a consistent change to
three separate places.

What's the ideal long-term outcome?  The reason why I suggested
during the v5 review an array of structure, a field in which can
be the .disabled field, was because it would allow us to later
extend the struct to help users.  Wouldn't it be nice if we can
do something like:

    $ git advice --list "^fetch"
    fetchShowForcedUpdates	enabled
    $ git advice --list --verbose fetchShowForcedUpdates
    fetchShowForcedUpdates	enabled
	"git fetch" by default spends cycles to compute which
	branches were forcibly updated, but it can be turned off.
	To avoid mistaken sense of safety, however, a reminder
	message is issued instead when it is turned off.  The
	reminder can be turned off with this advice key.

Such a future enhancement will become possible by assiciating a
help text for each advice key, and the data structure introduced
in <xmqqsgiymupf.fsf@gitster-ct.c.googlers.com> was meant to be
the beginning of it.

I dunno.