Skip to content

Commit c35941e

Browse files
committed
clean: do not use strbuf_split*() [part 1]
builtin/clean.c:parse_choice() is fed a single line of input, which is space or comma separated list of tokens, and a list of menu items. It parses the tokens into number ranges (e.g. 1-3 that means the first three items) or string prefix (e.g. 's' to choose the menu item "(s)elect") that specify the elements in the menu item list, and tells the caller which ones are chosen. For parsing the input string, it uses strbuf_split() to split it into bunch of strbufs. Instead use string_list_split_in_place(), for a few reasons. * strbuf_split() is a bad API function to use, that yields an array of strbuf that is a bad data structure to use in general. * string_list_split_in_place() allows you to split with "comma or space"; the current code has to preprocess the input string to replace comma with space because strbuf_split() does not allow this. Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5a8c069 commit c35941e

File tree

1 file changed

+23
-27
lines changed

1 file changed

+23
-27
lines changed

builtin/clean.c

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -480,40 +480,36 @@ static int parse_choice(struct menu_stuff *menu_stuff,
480480
struct strbuf *input,
481481
int **chosen)
482482
{
483-
struct strbuf **choice_list, **ptr;
483+
struct string_list choice = STRING_LIST_INIT_NODUP;
484+
struct string_list_item *item;
484485
int nr = 0;
485486
int i;
486487

487-
if (is_single) {
488-
choice_list = strbuf_split_max(input, '\n', 0);
489-
} else {
490-
char *p = input->buf;
491-
do {
492-
if (*p == ',')
493-
*p = ' ';
494-
} while (*p++);
495-
choice_list = strbuf_split_max(input, ' ', 0);
496-
}
488+
string_list_split_in_place_f(&choice, input->buf,
489+
is_single ? "\n" : ", ", -1,
490+
STRING_LIST_SPLIT_TRIM);
497491

498-
for (ptr = choice_list; *ptr; ptr++) {
499-
char *p;
500-
int choose = 1;
492+
for_each_string_list_item(item, &choice) {
493+
const char *string;
494+
int choose;
501495
int bottom = 0, top = 0;
502496
int is_range, is_number;
503497

504-
strbuf_trim(*ptr);
505-
if (!(*ptr)->len)
498+
string = item->string;
499+
if (!*string)
506500
continue;
507501

508502
/* Input that begins with '-'; unchoose */
509-
if (*(*ptr)->buf == '-') {
503+
if (string[0] == '-') {
510504
choose = 0;
511-
strbuf_remove((*ptr), 0, 1);
505+
string++;
506+
} else {
507+
choose = 1;
512508
}
513509

514510
is_range = 0;
515511
is_number = 1;
516-
for (p = (*ptr)->buf; *p; p++) {
512+
for (const char *p = string; *p; p++) {
517513
if ('-' == *p) {
518514
if (!is_range) {
519515
is_range = 1;
@@ -531,27 +527,27 @@ static int parse_choice(struct menu_stuff *menu_stuff,
531527
}
532528

533529
if (is_number) {
534-
bottom = atoi((*ptr)->buf);
530+
bottom = atoi(string);
535531
top = bottom;
536532
} else if (is_range) {
537-
bottom = atoi((*ptr)->buf);
533+
bottom = atoi(string);
538534
/* a range can be specified like 5-7 or 5- */
539-
if (!*(strchr((*ptr)->buf, '-') + 1))
535+
if (!*(strchr(string, '-') + 1))
540536
top = menu_stuff->nr;
541537
else
542-
top = atoi(strchr((*ptr)->buf, '-') + 1);
543-
} else if (!strcmp((*ptr)->buf, "*")) {
538+
top = atoi(strchr(string, '-') + 1);
539+
} else if (!strcmp(string, "*")) {
544540
bottom = 1;
545541
top = menu_stuff->nr;
546542
} else {
547-
bottom = find_unique((*ptr)->buf, menu_stuff);
543+
bottom = find_unique(string, menu_stuff);
548544
top = bottom;
549545
}
550546

551547
if (top <= 0 || bottom <= 0 || top > menu_stuff->nr || bottom > top ||
552548
(is_single && bottom != top)) {
553549
clean_print_color(CLEAN_COLOR_ERROR);
554-
printf(_("Huh (%s)?\n"), (*ptr)->buf);
550+
printf(_("Huh (%s)?\n"), string);
555551
clean_print_color(CLEAN_COLOR_RESET);
556552
continue;
557553
}
@@ -560,7 +556,7 @@ static int parse_choice(struct menu_stuff *menu_stuff,
560556
(*chosen)[i-1] = choose;
561557
}
562558

563-
strbuf_list_free(choice_list);
559+
string_list_clear(&choice, 0);
564560

565561
for (i = 0; i < menu_stuff->nr; i++)
566562
nr += (*chosen)[i];

0 commit comments

Comments
 (0)