Re: [PATCH v1 4/4] advice: remove static global variables for advice tracking

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Ben,

On Fri, 30 Jul 2021, Ben Boeckel wrote:

> All of the preferences are now tracked as part of the `advice_setting`
> array and all consumers of the global variables have been removed, so
> the parallel tracking through `advice_config` is no longer necessary.

Maybe add "This concludes the move from the old advice API to the new one
introduced via c4a09cc9ccb (Merge branch 'hw/advise-ng', 2020-03-25)"?

At least this reader would have found that helpful.

And as I mentioned in the review of patch 3/4, the removal of the
explicit assignment to the `advice_*` global variables should be moved
here.

Ciao,
Dscho

>
> Signed-off-by: Ben Boeckel <mathstuf@gmail.com>
> ---
>  advice.c | 77 --------------------------------------------------------
>  advice.h | 31 -----------------------
>  2 files changed, 108 deletions(-)
>
> diff --git a/advice.c b/advice.c
> index ee4edc5e28..6ba47f3f5e 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -4,37 +4,6 @@
>  #include "help.h"
>  #include "string-list.h"
>
> -int advice_fetch_show_forced_updates = 1;
> -int advice_push_update_rejected = 1;
> -int advice_push_non_ff_current = 1;
> -int advice_push_non_ff_matching = 1;
> -int advice_push_already_exists = 1;
> -int advice_push_fetch_first = 1;
> -int advice_push_needs_force = 1;
> -int advice_push_unqualified_ref_name = 1;
> -int advice_push_ref_needs_update = 1;
> -int advice_status_hints = 1;
> -int advice_status_u_option = 1;
> -int advice_status_ahead_behind_warning = 1;
> -int advice_commit_before_merge = 1;
> -int advice_reset_quiet_warning = 1;
> -int advice_resolve_conflict = 1;
> -int advice_sequencer_in_use = 1;
> -int advice_implicit_identity = 1;
> -int advice_detached_head = 1;
> -int advice_set_upstream_failure = 1;
> -int advice_object_name_warning = 1;
> -int advice_amworkdir = 1;
> -int advice_rm_hints = 1;
> -int advice_add_embedded_repo = 1;
> -int advice_ignored_hook = 1;
> -int advice_waiting_for_editor = 1;
> -int advice_graft_file_deprecated = 1;
> -int advice_checkout_ambiguous_remote_branch_name = 1;
> -int advice_submodule_alternate_error_strategy_die = 1;
> -int advice_add_ignored_file = 1;
> -int advice_add_empty_pathspec = 1;
> -
>  static int advice_use_color = -1;
>  static char advice_colors[][COLOR_MAXLEN] = {
>  	GIT_COLOR_RESET,
> @@ -62,45 +31,6 @@ static const char *advise_get_color(enum color_advice ix)
>  	return "";
>  }
>
> -static struct {
> -	const char *name;
> -	int *preference;
> -} advice_config[] = {
> -	{ "fetchShowForcedUpdates", &advice_fetch_show_forced_updates },
> -	{ "pushUpdateRejected", &advice_push_update_rejected },
> -	{ "pushNonFFCurrent", &advice_push_non_ff_current },
> -	{ "pushNonFFMatching", &advice_push_non_ff_matching },
> -	{ "pushAlreadyExists", &advice_push_already_exists },
> -	{ "pushFetchFirst", &advice_push_fetch_first },
> -	{ "pushNeedsForce", &advice_push_needs_force },
> -	{ "pushUnqualifiedRefName", &advice_push_unqualified_ref_name },
> -	{ "pushRefNeedsUpdate", &advice_push_ref_needs_update },
> -	{ "statusHints", &advice_status_hints },
> -	{ "statusUoption", &advice_status_u_option },
> -	{ "statusAheadBehindWarning", &advice_status_ahead_behind_warning },
> -	{ "commitBeforeMerge", &advice_commit_before_merge },
> -	{ "resetQuiet", &advice_reset_quiet_warning },
> -	{ "resolveConflict", &advice_resolve_conflict },
> -	{ "sequencerInUse", &advice_sequencer_in_use },
> -	{ "implicitIdentity", &advice_implicit_identity },
> -	{ "detachedHead", &advice_detached_head },
> -	{ "setUpstreamFailure", &advice_set_upstream_failure },
> -	{ "objectNameWarning", &advice_object_name_warning },
> -	{ "amWorkDir", &advice_amworkdir },
> -	{ "rmHints", &advice_rm_hints },
> -	{ "addEmbeddedRepo", &advice_add_embedded_repo },
> -	{ "ignoredHook", &advice_ignored_hook },
> -	{ "waitingForEditor", &advice_waiting_for_editor },
> -	{ "graftFileDeprecated", &advice_graft_file_deprecated },
> -	{ "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
> -	{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
> -	{ "addIgnoredFile", &advice_add_ignored_file },
> -	{ "addEmptyPathspec", &advice_add_empty_pathspec },
> -
> -	/* make this an alias for backward compatibility */
> -	{ "pushNonFastForward", &advice_push_update_rejected }
> -};
> -
>  static struct {
>  	const char *key;
>  	int enabled;
> @@ -228,13 +158,6 @@ int git_default_advice_config(const char *var, const char *value)
>  	if (!skip_prefix(var, "advice.", &k))
>  		return 0;
>
> -	for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
> -		if (strcasecmp(k, advice_config[i].name))
> -			continue;
> -		*advice_config[i].preference = git_config_bool(var, value);
> -		break;
> -	}
> -
>  	for (i = 0; i < ARRAY_SIZE(advice_setting); i++) {
>  		if (strcasecmp(k, advice_setting[i].key))
>  			continue;
> diff --git a/advice.h b/advice.h
> index 101c4054b7..17ee5d3633 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -5,37 +5,6 @@
>
>  struct string_list;
>
> -extern int advice_fetch_show_forced_updates;
> -extern int advice_push_update_rejected;
> -extern int advice_push_non_ff_current;
> -extern int advice_push_non_ff_matching;
> -extern int advice_push_already_exists;
> -extern int advice_push_fetch_first;
> -extern int advice_push_needs_force;
> -extern int advice_push_unqualified_ref_name;
> -extern int advice_push_ref_needs_update;
> -extern int advice_status_hints;
> -extern int advice_status_u_option;
> -extern int advice_status_ahead_behind_warning;
> -extern int advice_commit_before_merge;
> -extern int advice_reset_quiet_warning;
> -extern int advice_resolve_conflict;
> -extern int advice_sequencer_in_use;
> -extern int advice_implicit_identity;
> -extern int advice_detached_head;
> -extern int advice_set_upstream_failure;
> -extern int advice_object_name_warning;
> -extern int advice_amworkdir;
> -extern int advice_rm_hints;
> -extern int advice_add_embedded_repo;
> -extern int advice_ignored_hook;
> -extern int advice_waiting_for_editor;
> -extern int advice_graft_file_deprecated;
> -extern int advice_checkout_ambiguous_remote_branch_name;
> -extern int advice_submodule_alternate_error_strategy_die;
> -extern int advice_add_ignored_file;
> -extern int advice_add_empty_pathspec;
> -
>  /*
>   * To add a new advice, you need to:
>   * Define a new advice_type.
> --
> 2.31.1
>
>