Re: [PATCH v10 09/14] git-clean: use a git-add-interactive compatible UI
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:17
Jiang Xin [off-list ref] writes:
+static void print_highlight_menu_stuff(struct menu_stuff *stuff, int **chosen)
+{
+ struct string_list menu_list = STRING_LIST_INIT_DUP;
+ struct strbuf menu = STRBUF_INIT;
+ int i;
+
+ if (MENU_STUFF_TYPE_MENU_ITEM == stuff->type) {
+ ...
+ } else if (MENU_STUFF_TYPE_STRING_LIST == stuff->type) {
+ ...
+ }
This is better to write as:
switch (stuff->type) {
default:
die("programming error");
case MENU_STUFF_TYPE_MENU_ITEM:
...
break;
case MENU_STUFF_TYPE_STRING_LIST:
...
break;
}
Besides, there is no good reason to write an equality comparison
between constant and variable in that order (people call it a "Yoda
condition"); do "var == const" if you must.
Also please fix this one:
+ for_each_string_list_item(item, (struct string_list *)stuff->stuff) {
+ if ((*chosen)[i] < 0)
+ (*chosen)[i] = 0;
+ strbuf_addf(&menu, "%s%2d: %s", (*chosen)[i] ? "*" : " ", ++i, item->string);Because the evaluation order of function arguments are not defined (not left to right; these are comma-expressions), (*chosen)[i] ? "*" : " " may use the original value of "i", or value after increment the evaluation of ++i left in "i".