From 54a76bb9e556c6ccbba2081138aba434736c1b1c Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 27 Nov 2016 19:05:37 -0800 Subject: [PATCH] emit error message when test is given invalid int This augments the previous change for issue #3346 by adding an error message when an invalid integer is seen. This change is likely to be controversial so I'm not going to squash it into the previous change. --- share/functions/__fish_print_help.fish | 198 ++++++++++++------------- src/builtin_test.cpp | 35 +++-- tests/test4.in | 86 +++++------ 3 files changed, 162 insertions(+), 157 deletions(-) diff --git a/share/functions/__fish_print_help.fish b/share/functions/__fish_print_help.fish index 31190e552..30692ee3b 100644 --- a/share/functions/__fish_print_help.fish +++ b/share/functions/__fish_print_help.fish @@ -1,104 +1,104 @@ function __fish_print_help --description "Print help message for the specified fish function or builtin" --argument item - # special support for builtin_help_get() - set -l tty_width - if test "$item" = "--tty-width" - set tty_width $argv[2] - set item $argv[3] - end - - if test "$item" = '.' - set item source - end - - # Do nothing if the file does not exist - if not test -e "$__fish_datadir/man/man1/$item.1" -o -e "$__fish_datadir/man/man1/$item.1.gz" - return - end - - set -l IFS \n\ \t - - # Render help output, save output into the variable 'help' - set -l help - set -l cols - set -l rLL - if test "$tty_width" -gt 0 - set cols $tty_width - else if command test -t 1 - # We want to simulate `man`'s dynamic line length, because - # defaulting to 80 kind of sucks. - # Note: using `command test` instead of `test` because `test -t 1` - # doesn't seem to work right. - # Note: grab the size from the stdout terminal in case it's somehow - # different than the stdin of fish. - # use fd 3 to copy our stdout because we need to pipe the output of stty - begin - stty size 0<&3 | read __ cols - end 3<&1 - end - if test -n "$cols" - set cols (math $cols - 4) # leave a bit of space on the right - set rLL -rLL=$cols[1]n - end - set -lx GROFF_TMAC_PATH $__fish_datadir/groff - if test -e "$__fish_datadir/man/man1/$item.1" - set help (nroff -c -man -mfish -t $rLL "$__fish_datadir/man/man1/$item.1" ^/dev/null) - else if test -e "$__fish_datadir/man/man1/$item.1.gz" - set help (gunzip -c "$__fish_datadir/man/man1/$item.1.gz" ^/dev/null | nroff -c -man -mfish -t $rLL ^/dev/null) + # special support for builtin_help_get() + set -l tty_width 0 + if test "$item" = "--tty-width" + set tty_width $argv[2] + set item $argv[3] end - # The original implementation trimmed off the top 5 lines and bottom 3 lines - # from the nroff output. Perhaps that's reliable, but the magic numbers make - # me extremely nervous. Instead, let's just strip out any lines that start - # in the first column. "normal" manpages put all section headers in the first - # column, but fish manpages only leave NAME like that, which we want to trim - # away anyway. - # - # While we're at it, let's compress sequences of blank lines down to a single - # blank line, to duplicate the default behavior of `man`, or more accurately, - # the `-s` flag to `less` that `man` passes. - set -l state blank - for line in $help - # categorize the line - set -l line_type - switch $line - case ' *' \t\* - # starts with whitespace, check if it has non-whitespace - printf "%s\n" $line | read -l word __ - if test -n $word - set line_type normal - else - # lines with just spaces probably shouldn't happen - # but let's consider them to be blank - set line_type blank - end - case '' - set line_type blank - case '*' - # not leading space, and not empty, so must contain a non-space - # in the first column. That makes it a header/footer. - set line_type meta - end + if test "$item" = '.' + set item source + end - switch $state - case normal - switch $line_type - case normal - printf "%s\n" $line - case blank - set state blank - case meta - # skip it - end - case blank - switch $line_type - case normal - echo # print the blank line - printf "%s\n" $line - set state normal - case blank meta - # skip it - end - end - end | ul # post-process with `ul`, to interpret the old-style grotty escapes - echo # print a trailing blank line + # Do nothing if the file does not exist + if not test -e "$__fish_datadir/man/man1/$item.1" -o -e "$__fish_datadir/man/man1/$item.1.gz" + return + end + + set -l IFS \n\ \t + + # Render help output, save output into the variable 'help' + set -l help + set -l cols + set -l rLL + if test "$tty_width" -gt 0 + set cols $tty_width + else if command test -t 1 + # We want to simulate `man`'s dynamic line length, because + # defaulting to 80 kind of sucks. + # Note: using `command test` instead of `test` because `test -t 1` + # doesn't seem to work right. + # Note: grab the size from the stdout terminal in case it's somehow + # different than the stdin of fish. + # use fd 3 to copy our stdout because we need to pipe the output of stty + begin + stty size 0<&3 | read __ cols + end 3<&1 + end + if test -n "$cols" + set cols (math $cols - 4) # leave a bit of space on the right + set rLL -rLL=$cols[1]n + end + set -lx GROFF_TMAC_PATH $__fish_datadir/groff + if test -e "$__fish_datadir/man/man1/$item.1" + set help (nroff -c -man -mfish -t $rLL "$__fish_datadir/man/man1/$item.1" ^/dev/null) + else if test -e "$__fish_datadir/man/man1/$item.1.gz" + set help (gunzip -c "$__fish_datadir/man/man1/$item.1.gz" ^/dev/null | nroff -c -man -mfish -t $rLL ^/dev/null) + end + + # The original implementation trimmed off the top 5 lines and bottom 3 lines + # from the nroff output. Perhaps that's reliable, but the magic numbers make + # me extremely nervous. Instead, let's just strip out any lines that start + # in the first column. "normal" manpages put all section headers in the first + # column, but fish manpages only leave NAME like that, which we want to trim + # away anyway. + # + # While we're at it, let's compress sequences of blank lines down to a single + # blank line, to duplicate the default behavior of `man`, or more accurately, + # the `-s` flag to `less` that `man` passes. + set -l state blank + for line in $help + # categorize the line + set -l line_type + switch $line + case ' *' \t\* + # starts with whitespace, check if it has non-whitespace + printf "%s\n" $line | read -l word __ + if test -n $word + set line_type normal + else + # lines with just spaces probably shouldn't happen + # but let's consider them to be blank + set line_type blank + end + case '' + set line_type blank + case '*' + # not leading space, and not empty, so must contain a non-space + # in the first column. That makes it a header/footer. + set line_type meta + end + + switch $state + case normal + switch $line_type + case normal + printf "%s\n" $line + case blank + set state blank + case meta + # skip it + end + case blank + switch $line_type + case normal + echo # print the blank line + printf "%s\n" $line + set state normal + case blank meta + # skip it + end + end + end | ul # post-process with `ul`, to interpret the old-style grotty escapes + echo # print a trailing blank line end diff --git a/src/builtin_test.cpp b/src/builtin_test.cpp index f4fe816cb..2701389d4 100644 --- a/src/builtin_test.cpp +++ b/src/builtin_test.cpp @@ -4,6 +4,7 @@ #include "config.h" // IWYU pragma: keep #include +#include #include #include #include @@ -637,9 +638,13 @@ bool parenthetical_expression::evaluate(wcstring_list_t &errors) { // IEEE 1003.1 says nothing about what it means for two strings to be "algebraically equal". For // example, should we interpret 0x10 as 0, 10, or 16? Here we use only base 10 and use wcstoll, -// which allows for leading + and -, and leading whitespace. This matches bash. -static bool parse_number(const wcstring &arg, long long *out) { +// which allows for leading + and -, and whitespace. This is consistent, albeit a bit more lenient +// since we allow trailing whitespace, with other implementations such as bash. +static bool parse_number(const wcstring &arg, long long *out, wcstring_list_t &errors) { *out = fish_wcstoll(arg.c_str()); + if (errno) { + errors.push_back(format_string(_(L"invalid integer '%ls'"), arg.c_str())); + } return !errno; } @@ -655,28 +660,28 @@ static bool binary_primary_evaluate(test_expressions::token_t token, const wcstr return left != right; } case test_number_equal: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num == right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num == right_num; } case test_number_not_equal: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num != right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num != right_num; } case test_number_greater: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num > right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num > right_num; } case test_number_greater_equal: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num >= right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num >= right_num; } case test_number_lesser: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num < right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num < right_num; } case test_number_lesser_equal: { - return parse_number(left, &left_num) && parse_number(right, &right_num) && - left_num <= right_num; + return parse_number(left, &left_num, errors) && + parse_number(right, &right_num, errors) && left_num <= right_num; } default: { errors.push_back(format_string(L"Unknown token type in %s", __func__)); @@ -729,7 +734,7 @@ static bool unary_primary_evaluate(test_expressions::token_t token, const wcstri return !wstat(arg, &buf) && buf.st_size > 0; } case test_filedesc_t: { // "-t", whether the fd is associated with a terminal - return parse_number(arg, &num) && num == (int)num && isatty((int)num); + return parse_number(arg, &num, errors) && num == (int)num && isatty((int)num); } case test_fileperm_r: { // "-r", read permission return !waccess(arg, R_OK); diff --git a/tests/test4.in b/tests/test4.in index 6d5014553..08bc1be44 100644 --- a/tests/test4.in +++ b/tests/test4.in @@ -33,65 +33,65 @@ set -g smurf yellow call3 call4 -set -l foo 1 -set -g bar 2 -set -U baz 3 +set -l foo 1 +set -g bar 2 +set -U baz 3 -set -l -q foo +set -l -q foo if test $status -ne 0 - echo Test 5 fail + echo Test 5 fail else echo Test 5 pass end; -if not set -g -q bar - echo Test 6 fail +if not set -g -q bar + echo Test 6 fail else echo Test 6 pass end; -if not set -U -q baz - echo Test 7 fail +if not set -U -q baz + echo Test 7 fail else echo Test 7 pass end; -set -u -l -q foo -if test $status -ne 0 - echo Test 8 fail +set -u -l -q foo +if test $status -ne 0 + echo Test 8 fail else echo Test 8 pass end; -if not set -u -g -q bar - echo Test 9 fail +if not set -u -g -q bar + echo Test 9 fail else echo Test 9 pass end; -if not set -u -U -q baz - echo Test 10 fail +if not set -u -U -q baz + echo Test 10 fail else echo Test 10 pass end; -set -x -l -q foo +set -x -l -q foo if test $status -eq 0 - echo Test 11 fail + echo Test 11 fail else echo Test 11 pass end; -if set -x -g -q bar - echo Test 12 fail +if set -x -g -q bar + echo Test 12 fail else echo Test 12 pass end; -if set -x -U -q baz - echo Test 13 fail +if set -x -U -q baz + echo Test 13 fail else echo Test 13 pass end; @@ -100,61 +100,61 @@ set -x -l foo 1 set -x -g bar 2 set -x -U baz 3 -set -l -q foo -if test $status -ne 0 - echo Test 14 fail +set -l -q foo +if test $status -ne 0 + echo Test 14 fail else echo Test 14 pass end; -if not set -g -q bar - echo Test 15 fail +if not set -g -q bar + echo Test 15 fail else echo Test 15 pass end; -if not set -U -q baz - echo Test 16 fail +if not set -U -q baz + echo Test 16 fail else echo Test 16 pass end; -set -u -l -q foo -if test $status -ne 1 - echo Test 17 fail +set -u -l -q foo +if test $status -ne 1 + echo Test 17 fail else echo Test 17 pass end; -if set -u -g -q bar - echo Test 18 fail +if set -u -g -q bar + echo Test 18 fail else echo Test 18 pass end; -if set -u -U -q baz - echo Test 19 fail +if set -u -U -q baz + echo Test 19 fail else echo Test 19 pass end; -set -x -l -q foo -if test $status -ne 0 - echo Test 20 fail +set -x -l -q foo +if test $status -ne 0 + echo Test 20 fail else echo Test 20 pass end; -if not set -x -g -q bar - echo Test 21 fail +if not set -x -g -q bar + echo Test 21 fail else echo Test 21 pass end; -if not set -x -U -q baz - echo Test 22 fail +if not set -x -U -q baz + echo Test 22 fail else echo Test 22 pass end;