[PATCH 2/2] cmd: Rework of "choice" completed
Alexander Coffin
alexcoffin1999 at gmail.com
Tue Dec 18 14:07:21 CST 2018
From: alex <alexcoffin1999 at 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 at gmail.com>
---
programs/cmd/builtins.c | 456 +++++++++++++++++++++----------
programs/cmd/tests/test_builtins.cmd.exp | 22 +-
2 files changed, 323 insertions(+), 155 deletions(-)
diff --git a/programs/cmd/builtins.c b/programs/cmd/builtins.c
index 209de26..b16e9aa 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,338 @@ 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. */
+ 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;
+ 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;
+ 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);
+ if (c == EOF) {
+ error = 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;
+ }
+ }
}
- 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) {
+ error = TRUE;
+ 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 {
+ 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 1386467..7f4f7c4 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' ------------
- at todo_wine@Example message [A,B,C]?A
- at todo_wine@1
- at todo_wine@Example message [A,B,C]?B
- at todo_wine@2
- at todo_wine@[D,E,F]?F
- at todo_wine@3
+Example message [A,B,C]?A
+1
+Example message [A,B,C]?B
+2
+[D,E,F]?F
+3
@todo_wine@[A,B,C,X,Y,Z]?Y
@todo_wine at 5
- at todo_wine@A
- at todo_wine@1
- at todo_wine@[a,b,c,A,B,C]?A
- at todo_wine@4
- at todo_wine@255
+A
+1
+[a,b,c,A,B,C]?A
+4
+255
------------ Testing variable expansion ------------
~p0 should be path containing batch file
@path@
--
2.7.4
More information about the wine-devel
mailing list