help: cleanup the contruction of keys_uniq
construction of keys_uniq depends on sort operation executed on keys before processing, which does not gurantee that keys_uniq will be sorted. refactor the code to shift the sort operation after the processing to remove dependency on key's sort operation and strictly maintain the sorted order of keys_uniq. move strbuf init and release out of loop to reuse same buffer. dedent sort -u and sed in tests and replace grep with sed, to avoid piping grep's output to sed. Suggested-by: Siddharth Shrimali <r.siddharth.shrimali@gmail.com> Signed-off-by: Amisha Chhajed <amishhhaaaa@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Amisha Chhajed committed
Mar 12, 2026 at 00:54 UTC
088e994bf62a8135b87615c3e786958b397a9fd7
2 files changed
+78
-52
builtin/help.c
+56
-35
@@ -111,6 +111,49 @@ struct slot_expansion {
111
int found;
112
};
113
114
+static void set_config_vars(struct string_list *keys_uniq, struct string_list_item *var)
115
+{
116
+ struct strbuf sb = STRBUF_INIT;
117
+ const char *str = var->string;
118
+ const char *wildcard = strchr(str, '*');
119
+ const char *tag = strchr(str, '<');
120
+ const char *cut;
121
+
122
+ if (wildcard && tag)
123
+ cut = wildcard < tag ? wildcard : tag;
124
+ else if (wildcard)
125
+ cut = wildcard;
126
+ else if (tag)
127
+ cut = tag;
128
+ else {
129
+ string_list_append(keys_uniq, str);
130
+ return;
131
+ }
132
+
133
+ strbuf_add(&sb, str, cut - str);
134
+ string_list_append(keys_uniq, sb.buf);
135
+ strbuf_release(&sb);
136
+}
137
+
138
+static void set_config_sections(struct string_list *keys_uniq, struct string_list_item *var)
139
+{
140
+ struct strbuf sb = STRBUF_INIT;
141
+ const char *str = var->string;
142
+ const char *dot = strchr(str, '.');
143
+ const char *cut;
144
+
145
+ if (dot)
146
+ cut = dot;
147
+ else {
148
+ set_config_vars(keys_uniq, var);
149
+ return;
150
+ }
151
+
152
+ strbuf_add(&sb, str, cut - str);
153
+ string_list_append(keys_uniq, sb.buf);
154
+ strbuf_release(&sb);
155
+}
156
+
157
static void list_config_help(enum show_config_type type)
158
{
159
struct slot_expansion slot_expansions[] = {
@@ -131,13 +174,12 @@ static void list_config_help(enum show_config_type type)
174
struct string_list keys = STRING_LIST_INIT_DUP;
175
struct string_list keys_uniq = STRING_LIST_INIT_DUP;
176
struct string_list_item *item;
177
+ struct strbuf sb = STRBUF_INIT;
178
179
for (p = config_name_list; *p; p++) {
180
const char *var = *p;
137
- struct strbuf sb = STRBUF_INIT;
181
182
for (e = slot_expansions; e->prefix; e++) {
140
-
183
strbuf_reset(&sb);
184
strbuf_addf(&sb, "%s.%s", e->prefix, e->placeholder);
185
if (!strcasecmp(var, sb.buf)) {
@@ -146,60 +188,39 @@ static void list_config_help(enum show_config_type type)
188
break;
189
}
190
}
149
- strbuf_release(&sb);
191
+
192
if (!e->prefix)
193
string_list_append(&keys, var);
194
}
195
196
+ strbuf_release(&sb);
197
+
198
for (e = slot_expansions; e->prefix; e++)
199
if (!e->found)
200
BUG("slot_expansion %s.%s is not used",
201
e->prefix, e->placeholder);
202
159
- string_list_sort(&keys);
203
for (size_t i = 0; i < keys.nr; i++) {
161
- const char *var = keys.items[i].string;
162
- const char *wildcard, *tag, *cut;
163
- const char *dot = NULL;
164
- struct strbuf sb = STRBUF_INIT;
165
-
204
switch (type) {
205
case SHOW_CONFIG_HUMAN:
168
- puts(var);
169
- continue;
206
+ string_list_append(&keys_uniq, keys.items[i].string);
207
+ break;
208
case SHOW_CONFIG_SECTIONS:
171
- dot = strchr(var, '.');
209
+ set_config_sections(&keys_uniq, &keys.items[i]);
210
break;
211
case SHOW_CONFIG_VARS:
212
+ set_config_vars(&keys_uniq, &keys.items[i]);
213
break;
214
+ default:
215
+ BUG("%d: unexpected type", type);
216
}
176
- wildcard = strchr(var, '*');
177
- tag = strchr(var, '<');
178
-
179
- if (!dot && !wildcard && !tag) {
180
- string_list_append(&keys_uniq, var);
181
- continue;
182
- }
183
-
184
- if (dot)
185
- cut = dot;
186
- else if (wildcard && !tag)
187
- cut = wildcard;
188
- else if (!wildcard && tag)
189
- cut = tag;
190
- else
191
- cut = wildcard < tag ? wildcard : tag;
192
-
193
- strbuf_add(&sb, var, cut - var);
194
- string_list_append(&keys_uniq, sb.buf);
195
- strbuf_release(&sb);
196
-
217
}
198
- string_list_clear(&keys, 0);
199
- string_list_remove_duplicates(&keys_uniq, 0);
218
+
219
+ string_list_sort_u(&keys_uniq, 0);
220
for_each_string_list_item(item, &keys_uniq)
221
puts(item->string);
222
string_list_clear(&keys_uniq, 0);
223
+ string_list_clear(&keys, 0);
224
}
225
226
static enum help_format parse_help_format(const char *format)
t/t0012-help.sh
+22
-17
@@ -141,20 +141,23 @@ test_expect_success 'git help -c' '
141
142
'\''git help config'\'' for more information
143
EOF
144
- grep -v -E \
145
- -e "^[^.]+\.[^.]+$" \
146
- -e "^[^.]+\.[^.]+\.[^.]+$" \
147
- help.output >actual &&
144
+ sed -E -e "
145
+ /^[^.]+\.[^.]+$/d
146
+ /^[^.]+\.[^.]+\.[^.]+$/d
147
+ " help.output >actual &&
148
test_cmp expect actual
149
'
150
151
test_expect_success 'git help --config-for-completion' '
152
git help -c >human &&
153
- grep -E \
154
- -e "^[^.]+\.[^.]+$" \
155
- -e "^[^.]+\.[^.]+\.[^.]+$" human |
156
- sed -e "s/\*.*//" -e "s/<.*//" |
157
- sort -u >human.munged &&
153
+ sed -E -e "
154
+ /^[^.]+\.[^.]+$/b out
155
+ /^[^.]+\.[^.]+\.[^.]+$/b out
156
+ d
157
+ : out
158
+ s/\*.*//
159
+ s/<.*//
160
+ " human | sort -u >human.munged &&
161
162
git help --config-for-completion >vars &&
163
test_cmp human.munged vars
@@ -162,14 +165,16 @@ test_expect_success 'git help --config-for-completion' '
165
166
test_expect_success 'git help --config-sections-for-completion' '
167
git help -c >human &&
165
- grep -E \
166
- -e "^[^.]+\.[^.]+$" \
167
- -e "^[^.]+\.[^.]+\.[^.]+$" human |
168
- sed -e "s/\..*//" |
169
- sort -u >human.munged &&
170
-
171
- git help --config-sections-for-completion >sections &&
172
- test_cmp human.munged sections
168
+ sed -E -e "
169
+ /^[^.]+\.[^.]+$/b out
170
+ /^[^.]+\.[^.]+\.[^.]+$/b out
171
+ d
172
+ : out
173
+ s/\..*//
174
+ " human | sort -u >expect &&
175
+
176
+ git help --config-sections-for-completion >actual &&
177
+ test_cmp expect actual
178
'
179
180
test_section_spacing () {