From 06afcb43b4aa2674c04297cf3b4bd9c9c6a3f801 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 16 Sep 2017 13:50:47 -0500 Subject: [PATCH 1/7] Support reading to stdout via builtin `read -` Added an option to read to stdout via `read -`. While it may seem useless at first blush, it lets you do things like include mysql -p(read --silent) ... Without needing to save to a local variable and then echo it back. Kicks in when `-` is provided as the variable name to read to. This is in keeping with the de facto syntax for reading/writing from/to stdin/stdout instead of a file in, e.g., tar, cat, and other standard unix utilities. --- src/builtin_read.cpp | 41 +++++++++++++++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 6f75537d9..4361f1801 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -2,6 +2,7 @@ #include "config.h" // IWYU pragma: keep #include +#include #include #include #include @@ -374,6 +375,10 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg // Verify all variable names. for (int i = 0; i < argc; i++) { if (!valid_var_name(argv[i])) { + if (wcscmp(L"-", argv[i]) == 0) { + //writing to stdout instead + continue; + } streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, argv[i]); builtin_print_help(parser, streams, cmd, streams.err); return STATUS_INVALID_ARGS; @@ -383,6 +388,26 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg return STATUS_CMD_OK; } +void print_or_set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t vals) { + if (key == L"-") { + for (const auto &val : vals) { + std::wcout << val << std::endl; + } + } + else { + env_set(key, mode, vals); + } +} + +void print_or_set_one(const wcstring &key, env_mode_flags_t mode, wcstring val) { + if (key == L"-") { + std::wcout << val << std::endl; + } + else { + env_set_one(key, mode, val); + } +} + /// The read builtin. Reads from stdin and stores the values in environment variables. int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wchar_t *cmd = argv[0]; @@ -455,7 +480,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (opts.array) { // Array mode: assign each char as a separate element of the sole var. - env_set(argv[0], opts.place, chars); + print_or_set(argv[0], opts.place, chars); } else { // Not array mode: assign each char to a separate var with the remainder being assigned // to the last var. @@ -463,16 +488,16 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { size_t j = 0; for (; i + 1 < argc; ++i) { if (j < chars.size()) { - env_set_one(argv[i], opts.place, chars[j]); + print_or_set_one(argv[i], opts.place, chars[j]); j++; } else { - env_set_one(argv[i], opts.place, L""); + print_or_set_one(argv[i], opts.place, L""); } } if (i < argc) { wcstring val = chars.size() == static_cast(argc) ? chars[i] : L""; - env_set_one(argv[i], opts.place, val); + print_or_set_one(argv[i], opts.place, val); } else { env_set_empty(argv[i], opts.place); } @@ -490,13 +515,13 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { loc.first != wcstring::npos; loc = wcstring_tok(buff, opts.delimiter, loc)) { tokens.emplace_back(wcstring(buff, loc.first, loc.second)); } - env_set(argv[0], opts.place, tokens); + print_or_set(argv[0], opts.place, tokens); } else { // We're using a delimiter provided by the user so use the `string split` behavior. wcstring_list_t splits; split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, LONG_MAX); - env_set(argv[0], opts.place, splits); + print_or_set(argv[0], opts.place, splits); } } else { // Not array mode. Split the input into tokens and assign each to the vars in sequence. @@ -510,7 +535,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (loc.first != wcstring::npos) { substr = wcstring(buff, loc.first, loc.second); } - env_set_one(argv[i], opts.place, substr); + print_or_set_one(argv[i], opts.place, substr); } } else { // We're using a delimiter provided by the user so use the `string split` behavior. @@ -520,7 +545,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, argc - 1); for (size_t i = 0; i < (size_t)argc && i < splits.size(); i++) { - env_set_one(argv[i], opts.place, splits[i]); + print_or_set_one(argv[i], opts.place, splits[i]); } } } From ec56b632f5d82587d28ac645de894f59cc9c49df Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 16 Sep 2017 14:00:33 -0500 Subject: [PATCH 2/7] Add read to stdout improvement to README.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f7834fabb..786be2684 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,7 @@ This section is for changes merged to the `major` branch that are not also merge - `math` is now a builtin rather than a wrapper around `bc` (#3157). - `history search` supports globs for wildcard searching (#3136). - `bind` has a new `--silent` option to ignore bind requests for named keys not available under the current `$TERMINAL` (#4188, #4431) +- `read` can write directly to stdout if `-` is supplied as a variable name (#4407) ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). From d0bc04cb4088a23633d775da1c81691cb4b1fb70 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 7 Oct 2017 00:15:04 +0200 Subject: [PATCH 3/7] Change read to stdout behavior to kick in on no arguments only No longer using `-` to indicate reading to stdout. Use lack of arguments as stdout indicator. This prevents mixing of variables with stdout reading and makes it clear that stdout may not be mixed with delimiters or array mode. --- src/builtin_read.cpp | 52 +++++++++++++++++++------------------------- 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 4361f1801..84fc74c8f 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -48,6 +48,7 @@ struct read_cmd_opts_t { bool array = false; bool silent = false; bool split_null = false; + bool stdout = false; int nchars = 0; }; @@ -73,6 +74,11 @@ static const struct woption long_options[] = {{L"export", no_argument, NULL, 'x' static int parse_cmd_opts(read_cmd_opts_t &opts, int *optind, //!OCLINT(high ncss method) int argc, wchar_t **argv, parser_t &parser, io_streams_t &streams) { + if (argc == 1) { + opts.stdout = true; + return STATUS_CMD_OK; + } + wchar_t *cmd = argv[0]; int opt; wgetopter_t w; @@ -375,10 +381,6 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg // Verify all variable names. for (int i = 0; i < argc; i++) { if (!valid_var_name(argv[i])) { - if (wcscmp(L"-", argv[i]) == 0) { - //writing to stdout instead - continue; - } streams.err.append_format(BUILTIN_ERR_VARNAME, cmd, argv[i]); builtin_print_help(parser, streams, cmd, streams.err); return STATUS_INVALID_ARGS; @@ -388,24 +390,9 @@ static int validate_read_args(const wchar_t *cmd, read_cmd_opts_t &opts, int arg return STATUS_CMD_OK; } -void print_or_set(const wcstring &key, env_mode_flags_t mode, wcstring_list_t vals) { - if (key == L"-") { - for (const auto &val : vals) { - std::wcout << val << std::endl; - } - } - else { - env_set(key, mode, vals); - } -} - -void print_or_set_one(const wcstring &key, env_mode_flags_t mode, wcstring val) { - if (key == L"-") { - std::wcout << val << std::endl; - } - else { - env_set_one(key, mode, val); - } +void write_stdout(wcstring val) { + const std::string &out = wcs2string(val); + write_loop(STDOUT_FILENO, out.c_str(), out.size()); } /// The read builtin. Reads from stdin and stores the values in environment variables. @@ -452,6 +439,11 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { return exit_res; } + if (opts.stdout) { + write_stdout(buff); + return exit_res; + } + if (!opts.have_delimiter) { auto ifs = env_get(L"IFS"); if (!ifs.missing_or_empty()) opts.delimiter = ifs->as_string(); @@ -480,7 +472,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (opts.array) { // Array mode: assign each char as a separate element of the sole var. - print_or_set(argv[0], opts.place, chars); + env_set(argv[0], opts.place, chars); } else { // Not array mode: assign each char to a separate var with the remainder being assigned // to the last var. @@ -488,16 +480,16 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { size_t j = 0; for (; i + 1 < argc; ++i) { if (j < chars.size()) { - print_or_set_one(argv[i], opts.place, chars[j]); + env_set_one(argv[i], opts.place, chars[j]); j++; } else { - print_or_set_one(argv[i], opts.place, L""); + env_set_one(argv[i], opts.place, L""); } } if (i < argc) { wcstring val = chars.size() == static_cast(argc) ? chars[i] : L""; - print_or_set_one(argv[i], opts.place, val); + env_set_one(argv[i], opts.place, val); } else { env_set_empty(argv[i], opts.place); } @@ -515,13 +507,13 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { loc.first != wcstring::npos; loc = wcstring_tok(buff, opts.delimiter, loc)) { tokens.emplace_back(wcstring(buff, loc.first, loc.second)); } - print_or_set(argv[0], opts.place, tokens); + env_set(argv[0], opts.place, tokens); } else { // We're using a delimiter provided by the user so use the `string split` behavior. wcstring_list_t splits; split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, LONG_MAX); - print_or_set(argv[0], opts.place, splits); + env_set(argv[0], opts.place, splits); } } else { // Not array mode. Split the input into tokens and assign each to the vars in sequence. @@ -535,7 +527,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { if (loc.first != wcstring::npos) { substr = wcstring(buff, loc.first, loc.second); } - print_or_set_one(argv[i], opts.place, substr); + env_set_one(argv[i], opts.place, substr); } } else { // We're using a delimiter provided by the user so use the `string split` behavior. @@ -545,7 +537,7 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) { split_about(buff.begin(), buff.end(), opts.delimiter.begin(), opts.delimiter.end(), &splits, argc - 1); for (size_t i = 0; i < (size_t)argc && i < splits.size(); i++) { - print_or_set_one(argv[i], opts.place, splits[i]); + env_set_one(argv[i], opts.place, splits[i]); } } } From 109769a147a30c9825330e3a77a41fc21ec463bf Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 7 Oct 2017 00:19:47 +0200 Subject: [PATCH 4/7] Update Changelog.md to reflect new `read` builtin behavior --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 786be2684..9ded345a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ This section is for changes merged to the `major` branch that are not also merge - `math` is now a builtin rather than a wrapper around `bc` (#3157). - `history search` supports globs for wildcard searching (#3136). - `bind` has a new `--silent` option to ignore bind requests for named keys not available under the current `$TERMINAL` (#4188, #4431) -- `read` can write directly to stdout if `-` is supplied as a variable name (#4407) +- `read` writes directly to stdout if called without arguments (#4407) ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). From 4780b28a935db166a60d29afba0472270fc2a429 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 7 Oct 2017 00:34:07 +0200 Subject: [PATCH 5/7] Updated `read` docs to include new stdout behavior --- doc_src/read.txt | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/doc_src/read.txt b/doc_src/read.txt index 636183363..f3c886076 100644 --- a/doc_src/read.txt +++ b/doc_src/read.txt @@ -7,10 +7,7 @@ read [OPTIONS] VARIABLES... \subsection read-description Description -`read` reads from standard input and stores the result in one or more shell variables. By default, -one line (terminated by a newline) is read into each variable. Alternatively, a null character or a -maximum number of characters can be used to terminate the input. Unlike other shells, there is no -default variable (such as `REPLY`) for storing the result. +`read` reads from standard input and either writes the result back to the terminal for use in command substitution or stores the result in one or more shell variables. By default, one line (terminated by a newline) is read into each variable. Alternatively, a null character or a maximum number of characters can be used to terminate the input. Unlike other shells, there is no default variable (such as `REPLY`) for storing the result. The following options are available: @@ -48,6 +45,12 @@ The following options are available: `read` reads a single line of input from stdin, breaks it into tokens based on the delimiter set via `-d`/`--delimiter` as a complete string (like `string split` or, if that has not been given the (deprecated) `IFS` shell variable as a set of characters, and then assigns one token to each variable specified in `VARIABLES`. If there are more tokens than variables, the complete remainder is assigned to the last variable. As a special case, if `IFS` is set to the empty string, each character of the input is considered a separate token. +If no parameters are provided, `read` enters a special case that simply provides redirection from `stdin` to `stdout`, useful for command substitution. For instance, the fish shell command below can be used to read data that should be provided via a command line argument from the console instead of hardcoding it in the command itself, allowing the command to both be reused as-is in various contexts with different input values and preventing possibly sensitive text from being included in the shell history: + +`mysql -uuser -p(read)` + +When running in stdout redirect mode, `read` does not split the input in any way and text is redirected to standard output without any further processing or manipulation. + If `-a` or `--array` is provided, only one variable name is allowed and the tokens are stored as an array in this variable. See the documentation for `set` for more details on the scoping rules for variables. From 8ffc3ab242303b9d995a1633c4ba9e55096f29df Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Sat, 7 Oct 2017 00:35:52 +0200 Subject: [PATCH 6/7] Drop no-longer-needed iostream header from src/builtin_read.cpp --- src/builtin_read.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/builtin_read.cpp b/src/builtin_read.cpp index 84fc74c8f..2f7b8bcfd 100644 --- a/src/builtin_read.cpp +++ b/src/builtin_read.cpp @@ -2,7 +2,6 @@ #include "config.h" // IWYU pragma: keep #include -#include #include #include #include From 101ada83cbac3e51280839e40e9642e44314edda Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 10 Oct 2017 08:34:50 +0200 Subject: [PATCH 7/7] Fix unit tests for read to stdout behavior --- tests/read.err | 3 +-- tests/read.expect | 5 +++++ tests/read.in | 2 +- tests/read.out | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/read.err b/tests/read.err index d76028bc0..f7f4e2a6a 100644 --- a/tests/read.err +++ b/tests/read.err @@ -1,7 +1,6 @@ #################### -# Read with no vars is an error -read: Expected at least 1 args, got only 0 +# Read with no vars is not an error #################### # Read with -a and anything other than exactly on var name is an error diff --git a/tests/read.expect b/tests/read.expect index d26153ea5..3b04c2f3e 100644 --- a/tests/read.expect +++ b/tests/read.expect @@ -16,6 +16,11 @@ expect_prompt # read +send_line "read" +expect_read_prompt +send_line "text" +expect_prompt + send_line "read foo" expect_read_prompt send_line "text" diff --git a/tests/read.in b/tests/read.in index 50d28df3b..4561aa987 100644 --- a/tests/read.in +++ b/tests/read.in @@ -3,7 +3,7 @@ # Test read builtin and IFS. # -logmsg Read with no vars is an error +logmsg Read with no vars is not an error read logmsg Read with -a and anything other than exactly on var name is an error diff --git a/tests/read.out b/tests/read.out index eb1c23a6d..5dd8f66a2 100644 --- a/tests/read.out +++ b/tests/read.out @@ -1,6 +1,6 @@ #################### -# Read with no vars is an error +# Read with no vars is not an error #################### # Read with -a and anything other than exactly on var name is an error