From: alex alexcoffin1999@gmail.com
Almost completely reworked the currently implementation of "choice". The previous implementation had little worth reusing as it failed to pass anything but the most basic tests, and therefore was only used for a few of the new lines of code.
The only failing test does not working due to pipes being unimplemented.
Signed-off-by: Alexander Coffin alexcoffin1999@gmail.com --- programs/cmd/builtins.c | 473 +++++++++++++++++++++---------- programs/cmd/tests/test_builtins.cmd.exp | 22 +- 2 files changed, 340 insertions(+), 155 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c index 209de26..f464d8e 100644 --- a/programs/cmd/builtins.c +++ b/programs/cmd/builtins.c @@ -28,6 +28,7 @@
#define WIN32_LEAN_AND_MEAN
+#include <assert.h> #include "wcmd.h" #include <shellapi.h> #include "wine/debug.h" @@ -261,171 +262,355 @@ void WCMD_change_tty (void) {
}
+/* Returns a pointer to the option's value and increments arg_i appropriately */ +const WCHAR* WCMD_parse_option (const WCHAR** args, int args_size, int* arg_i) { + /* Trust that the first character was a slash */ + const WCHAR* arg = args[*arg_i]; + while (TRUE) { + if (*arg == '\0') { + if (*arg_i < args_size - 1) { + ++(*arg_i); + return args[*arg_i]; + } else { + return NULL; + } + } else if (*arg == ':') { + ++arg; + return arg; + } + ++arg; + } +} + +BOOL WCMD_update_valid_choice (const WCHAR* choices, char c, const BOOL case_sensitive, WCHAR* choice_selected) { + const WCHAR bell[] = { '\a', 0 }; + if (!case_sensitive) { + /* choices should already be all uppercase if case_sensitive is false */ + c = toupperW(c); + } + if (strchrW(choices, c) != NULL) { + WCHAR c_print[3]; + c_print[0] = c; + c_print[1] = '\n'; + c_print[2] = '\0'; + WCMD_output_asis(c_print); + *choice_selected = c; + return TRUE; + } else { + /* key not allowed: play the bell */ + WINE_TRACE("key not in choices: %c\n", c); + WCMD_output_asis(bell); + return FALSE; + } +} + +int WCMD_choice_add_opt_c (const WCHAR* choices_input, WCHAR* choices, size_t choices_size) { + /* Windows only allows alphanumeric characters and 128-256 valued extended + ascii. This limitation seems unnecessary and it also is hard to check + in a platform-independent manner. */ + WINE_TRACE("Attempting to add choices: %s\n", choices_input); + size_t choices_len = strlenW(choices); + while (*choices_input != '\0') { + WCHAR c = *choices_input; + if (strchrW(choices, c) == NULL && choices_len < choices_size - 1) { + choices[choices_len] = c; + choices[choices_len + 1] = '\0'; + ++choices_len; + } else { + /* Only one of each option is allowed */ + return -1; + } + ++choices_input; + } + return 0; +} + /**************************************************************************** * WCMD_choice * */
-void WCMD_choice (const WCHAR * args) { - - static const WCHAR bellW[] = {7,0}; - static const WCHAR commaW[] = {',',0}; - static const WCHAR bracket_open[] = {'[',0}; - static const WCHAR bracket_close[] = {']','?',0}; - WCHAR answer[16]; - WCHAR buffer[16]; - WCHAR *ptr = NULL; - WCHAR *opt_c = NULL; - WCHAR *my_command = NULL; - WCHAR opt_default = 0; - DWORD opt_timeout = 0; - DWORD count; - DWORD oldmode; - BOOL have_console; - BOOL opt_n = FALSE; - BOOL opt_s = FALSE; - - have_console = GetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), &oldmode); - errorlevel = 0; - - my_command = heap_strdupW(WCMD_skip_leading_spaces((WCHAR*) args)); - - ptr = WCMD_skip_leading_spaces(my_command); - while (*ptr == '/') { - switch (toupperW(ptr[1])) { - case 'C': - ptr += 2; - /* the colon is optional */ - if (*ptr == ':') - ptr++; - - if (!*ptr || isspaceW(*ptr)) { - WINE_FIXME("bad parameter %s for /C\n", wine_dbgstr_w(ptr)); - heap_free(my_command); - return; - } - - /* remember the allowed keys (overwrite previous /C option) */ - opt_c = ptr; - while (*ptr && (!isspaceW(*ptr))) - ptr++; - - if (*ptr) { - /* terminate allowed chars */ - *ptr = 0; - ptr = WCMD_skip_leading_spaces(&ptr[1]); - } - WINE_TRACE("answer-list: %s\n", wine_dbgstr_w(opt_c)); - break; - - case 'N': - opt_n = TRUE; - ptr = WCMD_skip_leading_spaces(&ptr[2]); - break; - - case 'S': - opt_s = TRUE; - ptr = WCMD_skip_leading_spaces(&ptr[2]); - break; - - case 'T': - ptr = &ptr[2]; - /* the colon is optional */ - if (*ptr == ':') - ptr++; - - opt_default = *ptr++; +void WCMD_choice (const WCHAR* args) { + const WCHAR bracket_open[] = {'[',0}; + const WCHAR bracket_close[] = {']','?',0}; + const WCHAR space_wchar[] = {' ',0}; + DWORD old_console_mode; + BOOL have_console; + BOOL error = FALSE; + /* CTRL+c or otherwise */ + BOOL user_canceled = FALSE; + int parsed_args_size; + WCHAR** parsed_args; + WCHAR default_choice; + WCHAR choice_selected; + /* Only extended ASCII characters are allowed */ + WCHAR choices[256]; + const size_t choices_size = sizeof(choices) / sizeof(WCHAR); + BOOL show_choices = TRUE; + BOOL case_sensitive = FALSE; + WCHAR* message = NULL; + int timeout_secs = -1; + /* options may be only specified once */ + BOOL opt_c = FALSE; + BOOL opt_d = FALSE; + BOOL opt_m = FALSE; + BOOL opt_t = FALSE; + + /* to make GCC happy, clang is smart + part 1/2. It incorrectly thinks there + is a "maybe uninitialized" value */ + default_choice = '\0'; + + choices[0] = '\0'; + parsed_args = CommandLineToArgvW(args, &parsed_args_size); + + have_console = GetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), &old_console_mode); + if (have_console) { + /* Don't automatically echo the user's input */ + SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), 0); + }
- if (!opt_default || (*ptr != ',')) { - WINE_FIXME("bad option %s for /T\n", opt_default ? wine_dbgstr_w(ptr) : ""); - heap_free(my_command); - return; + if (parsed_args == NULL) { + /* Out of memory? */ + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_SYNTAXERR)); + } else { + for (int arg_i = 0; arg_i < parsed_args_size; ++arg_i) { + /* Yes, CHOICE appears to not skip whitespace, e.g. choice " /C:ABC" is invalid */ + if (toupperW(parsed_args[arg_i][0]) == '/') { + switch (toupperW(parsed_args[arg_i][1])) { + case 'C': + if (toupperW(parsed_args[arg_i][2]) == 'S') { + case_sensitive = TRUE; + } else { + if (!opt_c) { + const WCHAR* opt_value = WCMD_parse_option((const WCHAR**) parsed_args, parsed_args_size, &arg_i); + opt_c = TRUE; + if (opt_value == NULL + /* Note: the following line adds the values to choices */ + || WCMD_choice_add_opt_c(opt_value, choices, choices_size) != 0 + || strlenW(choices) == 0) { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); } - ptr++; - - count = 0; - while (((answer[count] = *ptr)) && isdigitW(*ptr) && (count < 15)) { - count++; - ptr++; + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + } + break; + case 'N': + show_choices = FALSE; + break; + case 'T': + if (!opt_t) { + WCHAR* end; + long seconds; + const WCHAR* opt_value = WCMD_parse_option((const WCHAR**) parsed_args, parsed_args_size, &arg_i); + opt_t = TRUE; + if (opt_value != NULL) { + seconds = strtolW(opt_value, &end, 10); + /* Windows says 9999 is the max supported timeout *shrug* */ + if (end != opt_value && *end == '\0' && seconds >= 0 && seconds < 10000) { + timeout_secs = seconds; + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); } - - answer[count] = 0; - opt_timeout = atoiW(answer); - - ptr = WCMD_skip_leading_spaces(ptr); - break; - - default: - WINE_FIXME("bad parameter: %s\n", wine_dbgstr_w(ptr)); - heap_free(my_command); - return; + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + break; + case 'D': + if (!opt_d) { + const WCHAR* opt_value = WCMD_parse_option((const WCHAR**) parsed_args, parsed_args_size, &arg_i); + opt_d = TRUE; + if (opt_value != NULL) { + default_choice = opt_value[0]; + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + break; + case 'M': + if (!opt_m) { + const WCHAR* opt_value = WCMD_parse_option((const WCHAR**) parsed_args, parsed_args_size, &arg_i); + opt_m = TRUE; + if (opt_value != NULL) { + message = heap_strdupW(opt_value); + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + } else { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + break; + default: + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + break; } + } else { + /* All arguments have '/' as the first character or are directly after an option */ + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } }
- if (opt_timeout) - WINE_FIXME("timeout not supported: %c,%d\n", opt_default, opt_timeout); - - if (have_console) - SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), 0); + /* to make GCC happy, clang is smart + part 2/2 */ + if (opt_d && !error) { + assert(default_choice != '\0'); + }
- /* use default keys, when needed: localized versions of "Y"es and "No" */ + /* use default keys, when needed: localized versions of "Y"es and "N"o */ if (!opt_c) { - LoadStringW(hinst, WCMD_YES, buffer, ARRAY_SIZE(buffer)); - LoadStringW(hinst, WCMD_NO, buffer + 1, ARRAY_SIZE(buffer) - 1); - opt_c = buffer; - buffer[2] = 0; + LoadStringW(hinst, WCMD_YES, choices, choices_size); + LoadStringW(hinst, WCMD_NO, choices + 1, choices_size - 1); + choices[2] = '\0'; } - - /* print the question, when needed */ - if (*ptr) - WCMD_output_asis(ptr); - - if (!opt_s) { - struprW(opt_c); - WINE_TRACE("case insensitive answer-list: %s\n", wine_dbgstr_w(opt_c)); + if (!case_sensitive) { + WCHAR* c = choices; + while (*c != '\0') { + WCHAR tmp = *c; + *c = toupperW(*c); + if (*c != tmp) { + if (strchrW(c + 1, *c) != NULL) { + /* This means that there was already an uppercase version of *c. + There may only be one copy of each letter. */ + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + break; + } + } + ++c; + } + if (opt_d && !error) { + default_choice = toupperW(default_choice); + } } - - if (!opt_n) { - /* print a list of all allowed answers inside brackets */ + /* /D and /T have to be together */ + if (opt_d != opt_t) { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + if (opt_d && !error) { + if (strchrW(choices, default_choice) == NULL) { + error = TRUE; + WCMD_output_stderr(WCMD_LoadMessage(WCMD_ARGERR)); + } + } + if (!error) { + if (opt_m) { + WCMD_output_asis(message); + WCMD_output_asis(space_wchar); + } + if (show_choices) { + int choices_len; WCMD_output_asis(bracket_open); - ptr = opt_c; - answer[1] = 0; - while ((answer[0] = *ptr++)) { - WCMD_output_asis(answer); - if (*ptr) - WCMD_output_asis(commaW); + /* ERROR should be TRUE if there are 0 choices */ + assert(choices[0] != '\0'); + { + WCHAR print_out[2]; + print_out[0] = choices[0]; + print_out[1] = '\0'; + WCMD_output_asis(print_out); + } + /* safe to cast since because of choices_size */ + assert(choices_size == 256); + choices_len = (int) strlenW(choices); + for (int i = 1; i < choices_len; ++i) { + WCHAR print_out[3]; + print_out[0] = ','; + print_out[1] = choices[i]; + print_out[2] = '\0'; + WCMD_output_asis(print_out); } WCMD_output_asis(bracket_close); - } - - while (TRUE) { - - /* FIXME: Add support for option /T */ - answer[1] = 0; /* terminate single character string */ - WCMD_ReadFile(GetStdHandle(STD_INPUT_HANDLE), answer, 1, &count); - - if (!opt_s) - answer[0] = toupperW(answer[0]); - - ptr = strchrW(opt_c, answer[0]); - if (ptr) { - WCMD_output_asis(answer); - WCMD_output_asis(newlineW); - if (have_console) - SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), oldmode); - - errorlevel = (ptr - opt_c) + 1; - WINE_TRACE("answer: %d\n", errorlevel); - heap_free(my_command); - return; + } + assert(strlenW(choices) > 0); + if (opt_t && opt_d) { + if (timeout_secs == 0) { + BOOL status = WCMD_update_valid_choice(choices, default_choice, case_sensitive, &choice_selected); + /* default_choice should have already been validated */ + assert(status); + } else { + fd_set stdin_set; + struct timeval timeout_val; + BOOL eof_reached = FALSE; + FD_ZERO(&stdin_set); + FD_SET(fileno(stdin), &stdin_set); + timeout_val.tv_sec = timeout_secs; + timeout_val.tv_usec = 0; + while (TRUE) { + /* Yes, windows resets the timer each incorrect entry too. Although, + technically select() may modify the value to the reduced time + remaining. */ + int status = select(fileno(stdin) + 1, &stdin_set, NULL, NULL, &timeout_val); + if (status > 0) { + char c = getc(stdin); + /* A file will always be "ready" when it is at EOF + therefore to avoid a busy wait we will break. */ + if (c == EOF && feof(stdin) != 0) { + WINE_TRACE("EOF reached for choice with option /T. Sleeping instead...\n"); + eof_reached = TRUE; + break; + } else if (WCMD_update_valid_choice(choices, c, case_sensitive, &choice_selected)) { + break; + } + } else if (status == 0) { + BOOL status = WCMD_update_valid_choice(choices, default_choice, case_sensitive, &choice_selected); + assert(status); + break; + } + } + if (eof_reached) { + Sleep(timeout_secs * 1000); + BOOL status = WCMD_update_valid_choice(choices, default_choice, case_sensitive, &choice_selected); + assert(status); + } } - else - { - /* key not allowed: play the bell */ - WINE_TRACE("key not allowed: %s\n", wine_dbgstr_w(answer)); - WCMD_output_asis(bellW); + } else { + while (TRUE) { + char c = getc(stdin); + if (c == EOF) { + user_canceled = TRUE; + WINE_TRACE("EOF reached for user input without /T, treating as user cancelation\n"); + break; + } else if (WCMD_update_valid_choice(choices, c, case_sensitive, &choice_selected)) { + break; + } } + } } + LocalFree(parsed_args); + heap_free(message); + } + if (error) { + errorlevel = 255; + } else if (user_canceled) { + /* Windows sets the errorlevel to 0, although if a pipe is used which ends + it sets it to 255. *Shrug* pipes don't work in Wine yet anyway. */ + errorlevel = 0; + } else { + assert(strchrW(choices, choice_selected) != NULL); + errorlevel = (strchrW(choices, choice_selected) - choices) + 1; + } + if (have_console) { + /* Restore the previous console mode */ + SetConsoleMode(GetStdHandle(STD_INPUT_HANDLE), old_console_mode); + } }
/**************************************************************************** diff --git a/programs/cmd/tests/test_builtins.cmd.exp b/programs/cmd/tests/test_builtins.cmd.exp index d78d91a..2a80b39 100644 --- a/programs/cmd/tests/test_builtins.cmd.exp +++ b/programs/cmd/tests/test_builtins.cmd.exp @@ -480,19 +480,19 @@ I'm here!@space@ I'm here!@space@ I'm here!@space@ ------------ Testing 'choice' ------------ -@todo_wine@Example message [A,B,C]?A@or_broken@choice unavailable -@todo_wine@1@or_broken@9009 -@todo_wine@Example message [A,B,C]?B@or_broken@choice unavailable -@todo_wine@2@or_broken@9009 -@todo_wine@[D,E,F]?F@or_broken@choice unavailable -@todo_wine@3@or_broken@9009 +Example message [A,B,C]?A@or_broken@choice unavailable +1@or_broken@9009 +Example message [A,B,C]?B@or_broken@choice unavailable +2@or_broken@9009 +[D,E,F]?F@or_broken@choice unavailable +3@or_broken@9009 @todo_wine@[A,B,C,X,Y,Z]?Y@or_broken@choice unavailable @todo_wine@5@or_broken@9009 -@todo_wine@A@or_broken@choice unavailable -@todo_wine@1@or_broken@9009 -@todo_wine@[a,b,c,A,B,C]?A@or_broken@choice unavailable -@todo_wine@4@or_broken@9009 -@todo_wine@255@or_broken@9009 +A@or_broken@choice unavailable +1@or_broken@9009 +[a,b,c,A,B,C]?A@or_broken@choice unavailable +4@or_broken@9009 +255@or_broken@9009 ------------ Testing variable expansion ------------ ~p0 should be path containing batch file @path@