Skip to content

Commit c4a09cc

Browse files
gitstergitster
committed
Merge branch 'hw/advise-ng'
Revamping of the advise API to allow more systematic enumeration of advice knobs in the future. * hw/advise-ng: tag: use new advice API to check visibility advice: revamp advise API advice: change "setupStreamFailure" to "setUpstreamFailure" advice: extract vadvise() from advise()
2 parents 274b9cc + f665d63 commit c4a09cc

File tree

9 files changed

+200
-12
lines changed

9 files changed

+200
-12
lines changed

‎Makefile‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -695,6 +695,7 @@ X =
695695

696696
PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
697697

698+
TEST_BUILTINS_OBJS += test-advise.o
698699
TEST_BUILTINS_OBJS += test-chmtime.o
699700
TEST_BUILTINS_OBJS += test-config.o
700701
TEST_BUILTINS_OBJS += test-ctype.o

‎advice.c‎

Lines changed: 88 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ int advice_ignored_hook = 1;
2929
int advice_waiting_for_editor = 1;
3030
int advice_graft_file_deprecated = 1;
3131
int advice_checkout_ambiguous_remote_branch_name = 1;
32-
int advice_nested_tag = 1;
3332
int advice_submodule_alternate_error_strategy_die = 1;
3433
int advice_add_ignored_file = 1;
3534
int advice_add_empty_pathspec = 1;
@@ -82,7 +81,7 @@ static struct {
8281
{ "sequencerInUse", &advice_sequencer_in_use },
8382
{ "implicitIdentity", &advice_implicit_identity },
8483
{ "detachedHead", &advice_detached_head },
85-
{ "setupStreamFailure", &advice_set_upstream_failure },
84+
{ "setUpstreamFailure", &advice_set_upstream_failure },
8685
{ "objectNameWarning", &advice_object_name_warning },
8786
{ "amWorkDir", &advice_amworkdir },
8887
{ "rmHints", &advice_rm_hints },
@@ -91,7 +90,6 @@ static struct {
9190
{ "waitingForEditor", &advice_waiting_for_editor },
9291
{ "graftFileDeprecated", &advice_graft_file_deprecated },
9392
{ "checkoutAmbiguousRemoteBranchName", &advice_checkout_ambiguous_remote_branch_name },
94-
{ "nestedTag", &advice_nested_tag },
9593
{ "submoduleAlternateErrorStrategyDie", &advice_submodule_alternate_error_strategy_die },
9694
{ "addIgnoredFile", &advice_add_ignored_file },
9795
{ "addEmptyPathspec", &advice_add_empty_pathspec },
@@ -100,15 +98,58 @@ static struct {
10098
{ "pushNonFastForward", &advice_push_update_rejected }
10199
};
102100

103-
void advise(const char *advice, ...)
101+
static struct {
102+
const char *key;
103+
int enabled;
104+
} advice_setting[] = {
105+
[ADVICE_ADD_EMBEDDED_REPO] = { "addEmbeddedRepo", 1 },
106+
[ADVICE_AM_WORK_DIR] = { "amWorkDir", 1 },
107+
[ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME] = { "checkoutAmbiguousRemoteBranchName", 1 },
108+
[ADVICE_COMMIT_BEFORE_MERGE] = { "commitBeforeMerge", 1 },
109+
[ADVICE_DETACHED_HEAD] = { "detachedHead", 1 },
110+
[ADVICE_FETCH_SHOW_FORCED_UPDATES] = { "fetchShowForcedUpdates", 1 },
111+
[ADVICE_GRAFT_FILE_DEPRECATED] = { "graftFileDeprecated", 1 },
112+
[ADVICE_IGNORED_HOOK] = { "ignoredHook", 1 },
113+
[ADVICE_IMPLICIT_IDENTITY] = { "implicitIdentity", 1 },
114+
[ADVICE_NESTED_TAG] = { "nestedTag", 1 },
115+
[ADVICE_OBJECT_NAME_WARNING] = { "objectNameWarning", 1 },
116+
[ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists", 1 },
117+
[ADVICE_PUSH_FETCH_FIRST] = { "pushFetchFirst", 1 },
118+
[ADVICE_PUSH_NEEDS_FORCE] = { "pushNeedsForce", 1 },
119+
120+
/* make this an alias for backward compatibility */
121+
[ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward", 1 },
122+
123+
[ADVICE_PUSH_NON_FF_CURRENT] = { "pushNonFFCurrent", 1 },
124+
[ADVICE_PUSH_NON_FF_MATCHING] = { "pushNonFFMatching", 1 },
125+
[ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName", 1 },
126+
[ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected", 1 },
127+
[ADVICE_RESET_QUIET_WARNING] = { "resetQuiet", 1 },
128+
[ADVICE_RESOLVE_CONFLICT] = { "resolveConflict", 1 },
129+
[ADVICE_RM_HINTS] = { "rmHints", 1 },
130+
[ADVICE_SEQUENCER_IN_USE] = { "sequencerInUse", 1 },
131+
[ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure", 1 },
132+
[ADVICE_STATUS_AHEAD_BEHIND_WARNING] = { "statusAheadBehindWarning", 1 },
133+
[ADVICE_STATUS_HINTS] = { "statusHints", 1 },
134+
[ADVICE_STATUS_U_OPTION] = { "statusUoption", 1 },
135+
[ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE] = { "submoduleAlternateErrorStrategyDie", 1 },
136+
[ADVICE_WAITING_FOR_EDITOR] = { "waitingForEditor", 1 },
137+
};
138+
139+
static const char turn_off_instructions[] =
140+
N_("\n"
141+
"Disable this message with \"git config advice.%s false\"");
142+
143+
static void vadvise(const char *advice, int display_instructions,
144+
const char *key, va_list params)
104145
{
105146
struct strbuf buf = STRBUF_INIT;
106-
va_list params;
107147
const char *cp, *np;
108148

109-
va_start(params, advice);
110149
strbuf_vaddf(&buf, advice, params);
111-
va_end(params);
150+
151+
if (display_instructions)
152+
strbuf_addf(&buf, turn_off_instructions, key);
112153

113154
for (cp = buf.buf; *cp; cp = np) {
114155
np = strchrnul(cp, '\n');
@@ -122,6 +163,37 @@ void advise(const char *advice, ...)
122163
strbuf_release(&buf);
123164
}
124165

166+
void advise(const char *advice, ...)
167+
{
168+
va_list params;
169+
va_start(params, advice);
170+
vadvise(advice, 0, "", params);
171+
va_end(params);
172+
}
173+
174+
int advice_enabled(enum advice_type type)
175+
{
176+
switch(type) {
177+
case ADVICE_PUSH_UPDATE_REJECTED:
178+
return advice_setting[ADVICE_PUSH_UPDATE_REJECTED].enabled &&
179+
advice_setting[ADVICE_PUSH_UPDATE_REJECTED_ALIAS].enabled;
180+
default:
181+
return advice_setting[type].enabled;
182+
}
183+
}
184+
185+
void advise_if_enabled(enum advice_type type, const char *advice, ...)
186+
{
187+
va_list params;
188+
189+
if (!advice_enabled(type))
190+
return;
191+
192+
va_start(params, advice);
193+
vadvise(advice, 1, advice_setting[type].key, params);
194+
va_end(params);
195+
}
196+
125197
int git_default_advice_config(const char *var, const char *value)
126198
{
127199
const char *k, *slot_name;
@@ -148,6 +220,13 @@ int git_default_advice_config(const char *var, const char *value)
148220
if (strcasecmp(k, advice_config[i].name))
149221
continue;
150222
*advice_config[i].preference = git_config_bool(var, value);
223+
break;
224+
}
225+
226+
for (i = 0; i < ARRAY_SIZE(advice_setting); i++) {
227+
if (strcasecmp(k, advice_setting[i].key))
228+
continue;
229+
advice_setting[i].enabled = git_config_bool(var, value);
151230
return 0;
152231
}
153232

@@ -158,8 +237,8 @@ void list_config_advices(struct string_list *list, const char *prefix)
158237
{
159238
int i;
160239

161-
for (i = 0; i < ARRAY_SIZE(advice_config); i++)
162-
list_config_item(list, prefix, advice_config[i].name);
240+
for (i = 0; i < ARRAY_SIZE(advice_setting); i++)
241+
list_config_item(list, prefix, advice_setting[i].key);
163242
}
164243

165244
int error_resolve_conflict(const char *me)

‎advice.h‎

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,64 @@ extern int advice_ignored_hook;
2929
extern int advice_waiting_for_editor;
3030
extern int advice_graft_file_deprecated;
3131
extern int advice_checkout_ambiguous_remote_branch_name;
32-
extern int advice_nested_tag;
3332
extern int advice_submodule_alternate_error_strategy_die;
3433
extern int advice_add_ignored_file;
3534
extern int advice_add_empty_pathspec;
3635

36+
/*
37+
* To add a new advice, you need to:
38+
* Define a new advice_type.
39+
* Add a new entry to advice_setting array.
40+
* Add the new config variable to Documentation/config/advice.txt.
41+
* Call advise_if_enabled to print your advice.
42+
*/
43+
enum advice_type {
44+
ADVICE_ADD_EMBEDDED_REPO,
45+
ADVICE_AM_WORK_DIR,
46+
ADVICE_CHECKOUT_AMBIGUOUS_REMOTE_BRANCH_NAME,
47+
ADVICE_COMMIT_BEFORE_MERGE,
48+
ADVICE_DETACHED_HEAD,
49+
ADVICE_FETCH_SHOW_FORCED_UPDATES,
50+
ADVICE_GRAFT_FILE_DEPRECATED,
51+
ADVICE_IGNORED_HOOK,
52+
ADVICE_IMPLICIT_IDENTITY,
53+
ADVICE_NESTED_TAG,
54+
ADVICE_OBJECT_NAME_WARNING,
55+
ADVICE_PUSH_ALREADY_EXISTS,
56+
ADVICE_PUSH_FETCH_FIRST,
57+
ADVICE_PUSH_NEEDS_FORCE,
58+
ADVICE_PUSH_NON_FF_CURRENT,
59+
ADVICE_PUSH_NON_FF_MATCHING,
60+
ADVICE_PUSH_UNQUALIFIED_REF_NAME,
61+
ADVICE_PUSH_UPDATE_REJECTED_ALIAS,
62+
ADVICE_PUSH_UPDATE_REJECTED,
63+
ADVICE_RESET_QUIET_WARNING,
64+
ADVICE_RESOLVE_CONFLICT,
65+
ADVICE_RM_HINTS,
66+
ADVICE_SEQUENCER_IN_USE,
67+
ADVICE_SET_UPSTREAM_FAILURE,
68+
ADVICE_STATUS_AHEAD_BEHIND_WARNING,
69+
ADVICE_STATUS_HINTS,
70+
ADVICE_STATUS_U_OPTION,
71+
ADVICE_SUBMODULE_ALTERNATE_ERROR_STRATEGY_DIE,
72+
ADVICE_WAITING_FOR_EDITOR,
73+
};
74+
3775
int git_default_advice_config(const char *var, const char *value);
3876
__attribute__((format (printf, 1, 2)))
3977
void advise(const char *advice, ...);
78+
79+
/**
80+
* Checks if advice type is enabled (can be printed to the user).
81+
* Should be called before advise().
82+
*/
83+
int advice_enabled(enum advice_type type);
84+
85+
/**
86+
* Checks the visibility of the advice before printing.
87+
*/
88+
void advise_if_enabled(enum advice_type type, const char *advice, ...);
89+
4090
int error_resolve_conflict(const char *me);
4191
void NORETURN die_resolve_conflict(const char *me);
4292
void NORETURN die_conclude_merge(void);

‎builtin/tag.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,8 +231,9 @@ static void create_tag(const struct object_id *object, const char *object_ref,
231231
if (type <= OBJ_NONE)
232232
die(_("bad object type."));
233233

234-
if (type == OBJ_TAG && advice_nested_tag)
235-
advise(_(message_advice_nested_tag), tag, object_ref);
234+
if (type == OBJ_TAG)
235+
advise_if_enabled(ADVICE_NESTED_TAG, _(message_advice_nested_tag),
236+
tag, object_ref);
236237

237238
strbuf_addf(&header,
238239
"object %s\n"

‎t/helper/test-advise.c‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
#include "test-tool.h"
2+
#include "cache.h"
3+
#include "advice.h"
4+
#include "config.h"
5+
6+
int cmd__advise_if_enabled(int argc, const char **argv)
7+
{
8+
if (!argv[1])
9+
die("usage: %s <advice>", argv[0]);
10+
11+
setup_git_directory();
12+
git_config(git_default_config, NULL);
13+
14+
/*
15+
* Any advice type can be used for testing, but NESTED_TAG was
16+
* selected here and in t0018 where this command is being
17+
* executed.
18+
*/
19+
advise_if_enabled(ADVICE_NESTED_TAG, argv[1]);
20+
21+
return 0;
22+
}

‎t/helper/test-tool.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ struct test_cmd {
1414
};
1515

1616
static struct test_cmd cmds[] = {
17+
{ "advise", cmd__advise_if_enabled },
1718
{ "chmtime", cmd__chmtime },
1819
{ "config", cmd__config },
1920
{ "ctype", cmd__ctype },

‎t/helper/test-tool.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#define USE_THE_INDEX_COMPATIBILITY_MACROS
55
#include "git-compat-util.h"
66

7+
int cmd__advise_if_enabled(int argc, const char **argv);
78
int cmd__chmtime(int argc, const char **argv);
89
int cmd__config(int argc, const char **argv);
910
int cmd__ctype(int argc, const char **argv);

‎t/t0018-advice.sh‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
#!/bin/sh
2+
3+
test_description='Test advise_if_enabled functionality'
4+
5+
. ./test-lib.sh
6+
7+
test_expect_success 'advice should be printed when config variable is unset' '
8+
cat >expect <<-\EOF &&
9+
hint: This is a piece of advice
10+
hint: Disable this message with "git config advice.nestedTag false"
11+
EOF
12+
test-tool advise "This is a piece of advice" 2>actual &&
13+
test_i18ncmp expect actual
14+
'
15+
16+
test_expect_success 'advice should be printed when config variable is set to true' '
17+
cat >expect <<-\EOF &&
18+
hint: This is a piece of advice
19+
hint: Disable this message with "git config advice.nestedTag false"
20+
EOF
21+
test_config advice.nestedTag true &&
22+
test-tool advise "This is a piece of advice" 2>actual &&
23+
test_i18ncmp expect actual
24+
'
25+
26+
test_expect_success 'advice should not be printed when config variable is set to false' '
27+
test_config advice.nestedTag false &&
28+
test-tool advise "This is a piece of advice" 2>actual &&
29+
test_must_be_empty actual
30+
'
31+
32+
test_done

‎t/t7004-tag.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1726,6 +1726,7 @@ test_expect_success 'recursive tagging should give advice' '
17261726
hint: already a tag. If you meant to tag the object that it points to, use:
17271727
hint: |
17281728
hint: git tag -f nested annotated-v4.0^{}
1729+
hint: Disable this message with "git config advice.nestedTag false"
17291730
EOF
17301731
git tag -m nested nested annotated-v4.0 2>actual &&
17311732
test_i18ncmp expect actual

0 commit comments

Comments
 (0)