From 9ef47a43a4127c16df468ee24f488358073dfa2c Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 20 Jul 2017 17:54:06 -0700 Subject: [PATCH 01/18] change how `argparse` handles boolean flags When reporting whether a boolean flag was seen report the actual flags rather than a summary count. For example, if you have option spec `h/help` and we parse `-h --help -h` don't do the equivalent of `set _flag_h 3` do `set _flag_h -h --help -h`. Partial fix for #4226 --- doc_src/argparse.txt | 2 +- src/builtin_argparse.cpp | 8 ++++++++ tests/argparse.out | 16 ++++++++-------- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/doc_src/argparse.txt b/doc_src/argparse.txt index ae763382d..fc95946e0 100644 --- a/doc_src/argparse.txt +++ b/doc_src/argparse.txt @@ -13,7 +13,7 @@ Each OPTION_SPEC can be written in the domain specific language long_flag); + } assert(!w.woptarg); long_idx = -1; continue; diff --git a/tests/argparse.out b/tests/argparse.out index 71df1f721..eb3752ec8 100644 --- a/tests/argparse.out +++ b/tests/argparse.out @@ -2,8 +2,8 @@ # One arg and no matching flags argv help # Five args with two matching a flag -_flag_h 2 -_flag_help 2 +_flag_h '--help' '-h' +_flag_help '--help' '-h' argv 'help' 'me' 'a lot more' # Required, optional, and multiple flags _flag_a ABC @@ -12,23 +12,23 @@ _flag_d _flag_def _flag_g 'g1' 'g2' 'g3' _flag_ghk 'g1' 'g2' 'g3' -_flag_h 1 -_flag_help 1 +_flag_h --help +_flag_help --help argv 'help' 'me' # --stop-nonopt works _flag_a A2 _flag_abc A2 -_flag_h 1 -_flag_help 1 +_flag_h -h +_flag_help -h argv 'non-opt' 'second non-opt' '--help' # Implicit int flags work _flag_val 123 argv 'abc' 'def' _flag_t woohoo _flag_token woohoo -_flag_v 2 +_flag_v '-v' '--verbose' _flag_val -234 -_flag_verbose 2 +_flag_verbose '-v' '--verbose' argv 'a1' 'a2' # Should be set to 987 _flag_m 987 From f3130ce70b5aba97f6bf86c2174946d1c6ccc8bc Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Fri, 21 Jul 2017 15:55:52 -0700 Subject: [PATCH 02/18] fix `argparse` handling of short flag only specs @faho noticed that option specs which don't have a long flag name are not handled correctly. This fixes that and adds unit tests. Fixes #4232 --- src/builtin_argparse.cpp | 136 ++++++++++++++++++++------------------- tests/argparse.err | 2 + tests/argparse.in | 27 ++++++++ tests/argparse.out | 19 ++++++ 4 files changed, 119 insertions(+), 65 deletions(-) diff --git a/src/builtin_argparse.cpp b/src/builtin_argparse.cpp index 6d6518976..45475219d 100644 --- a/src/builtin_argparse.cpp +++ b/src/builtin_argparse.cpp @@ -181,8 +181,9 @@ static int parse_exclusive_args(argparse_cmd_opts_t &opts, io_streams_t &streams } static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, - const wcstring &option_spec, const wchar_t *s, + const wcstring &option_spec, const wchar_t **opt_spec_str, io_streams_t &streams) { + const wchar_t *s = *opt_spec_str; if (opt_spec->short_flag == opts.implicit_int_flag && *s && *s != L'!') { streams.err.append_format( _(L"%ls: Implicit int short flag '%lc' does not allow modifiers like '%lc'\n"), @@ -224,6 +225,7 @@ static bool parse_flag_modifiers(argparse_cmd_opts_t &opts, option_spec_t *opt_s } opts.options.emplace(opt_spec->short_flag, opt_spec); + *opt_spec_str = s; return true; } @@ -250,8 +252,18 @@ static bool parse_option_spec_sep(argparse_cmd_opts_t &opts, option_spec_t *opt_ } else if (*s == L'-') { opt_spec->short_flag_valid = false; s++; + if (!*s) { + streams.err.append_format(BUILTIN_ERR_INVALID_OPT_SPEC, opts.name.c_str(), + option_spec.c_str(), *(s - 1)); + return false; + } } else if (*s == L'/') { s++; // the struct is initialized assuming short_flag_valid should be true + if (!*s) { + streams.err.append_format(BUILTIN_ERR_INVALID_OPT_SPEC, opts.name.c_str(), + option_spec.c_str(), *(s - 1)); + return false; + } } else if (*s == L'#') { if (opts.implicit_int_flag) { streams.err.append_format(_(L"%ls: Implicit int flag '%lc' already defined\n"), @@ -261,10 +273,11 @@ static bool parse_option_spec_sep(argparse_cmd_opts_t &opts, option_spec_t *opt_ opts.implicit_int_flag = opt_spec->short_flag; opt_spec->num_allowed = 1; // mandatory arg and can appear only once s++; // the struct is initialized assuming short_flag_valid should be true + if (!*s) opts.options.emplace(opt_spec->short_flag, opt_spec); } else { - // Long flag name not allowed if second char isn't '/' or '-' so just check for + // Long flag name not allowed if second char isn't '/', '-' or '#' so just check for // behavior modifier chars. - return parse_flag_modifiers(opts, opt_spec, option_spec, s, streams); + if (!parse_flag_modifiers(opts, opt_spec, option_spec, &s, streams)) return false; } *opt_spec_str = s; @@ -272,8 +285,8 @@ static bool parse_option_spec_sep(argparse_cmd_opts_t &opts, option_spec_t *opt_ } /// This parses an option spec string into a struct option_spec. -static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, - io_streams_t &streams) { +static bool parse_option_spec(argparse_cmd_opts_t &opts, //!OCLINT(high npath complexity) + wcstring option_spec, io_streams_t &streams) { if (option_spec.empty()) { streams.err.append_format(_(L"%ls: An option spec must have a short flag letter\n"), opts.name.c_str()); @@ -288,25 +301,28 @@ static bool parse_option_spec(argparse_cmd_opts_t &opts, wcstring option_spec, } option_spec_t *opt_spec = new option_spec_t(*s++); + if (!*s) { + // Bool short flag only. + opts.options.emplace(opt_spec->short_flag, opt_spec); + return true; + } if (!parse_option_spec_sep(opts, opt_spec, option_spec, &s, streams)) return false; + if (!*s) return true; // parsed the entire string so the option spec doesn't have a long flag // Collect the long flag name. const wchar_t *e = s; while (*e && (*e == L'-' || *e == L'_' || iswalnum(*e))) e++; - if (e == s) { - streams.err.append_format(BUILTIN_ERR_INVALID_OPT_SPEC, opts.name.c_str(), - option_spec.c_str(), *(s - 1)); - return false; + if (e != s) { + opt_spec->long_flag = wcstring(s, e - s); + if (opts.long_to_short_flag.find(opt_spec->long_flag) != opts.long_to_short_flag.end()) { + streams.err.append_format(L"%ls: Long flag '%ls' already defined\n", opts.name.c_str(), + opt_spec->long_flag.c_str()); + return false; + } + opts.long_to_short_flag.emplace(opt_spec->long_flag, opt_spec->short_flag); } - opt_spec->long_flag = wcstring(s, e - s); - if (opts.long_to_short_flag.find(opt_spec->long_flag) != opts.long_to_short_flag.end()) { - streams.err.append_format(L"%ls: Long flag '%ls' already defined\n", opts.name.c_str(), - opt_spec->long_flag.c_str()); - return false; - } - opts.long_to_short_flag.emplace(opt_spec->long_flag, opt_spec->short_flag); - return parse_flag_modifiers(opts, opt_spec, option_spec, e, streams); + return parse_flag_modifiers(opts, opt_spec, option_spec, &e, streams); } static int collect_option_specs(argparse_cmd_opts_t &opts, int *optind, int argc, wchar_t **argv, @@ -434,21 +450,6 @@ static void populate_option_strings( long_options.get()[i] = {NULL, 0, NULL, 0}; } -// Add a count for how many times we saw each boolean flag but only if we saw the flag at least -// once. -static void update_bool_flag_counts(argparse_cmd_opts_t &opts) { - return; - for (auto it : opts.options) { - auto opt_spec = it.second; - // The '#' short flag is special. It doesn't take any values but isn't a boolean arg. - if (opt_spec->short_flag == L'#') continue; - if (opt_spec->num_allowed != 0 || opt_spec->num_seen == 0) continue; - wchar_t count[20]; - swprintf(count, sizeof count / sizeof count[0], L"%d", opt_spec->num_seen); - opt_spec->vals.push_back(wcstring(count)); - } -} - static int validate_arg(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, bool is_long_flag, const wchar_t *woptarg, io_streams_t &streams) { // Obviously if there is no arg validation command we assume the arg is okay. @@ -511,6 +512,42 @@ static int check_for_implicit_int(argparse_cmd_opts_t &opts, const wchar_t *val, return STATUS_CMD_OK; } +static int handle_flag(argparse_cmd_opts_t &opts, option_spec_t *opt_spec, int long_idx, + const wchar_t *woptarg, io_streams_t &streams) { + opt_spec->num_seen++; + if (opt_spec->num_allowed == 0) { + // It's a boolean flag. Save the flag we saw since it might be useful to know if the + // short or long flag was given. + assert(!woptarg); + if (long_idx == -1) { + opt_spec->vals.push_back(wcstring(1, L'-') + opt_spec->short_flag); + } else { + opt_spec->vals.push_back(L"--" + opt_spec->long_flag); + } + return STATUS_CMD_OK; + } + + if (woptarg) { + int retval = validate_arg(opts, opt_spec, long_idx != -1, woptarg, streams); + if (retval != STATUS_CMD_OK) return retval; + } + + if (opt_spec->num_allowed == -1 || opt_spec->num_allowed == 1) { + // We're depending on `wgetopt_long()` to report that a mandatory value is missing if + // `opt_spec->num_allowed == 1` and thus return ':' so that we don't take this branch if + // the mandatory arg is missing. + opt_spec->vals.clear(); + if (woptarg) { + opt_spec->vals.push_back(woptarg); + } + } else { + assert(woptarg); + opt_spec->vals.push_back(woptarg); + } + + return STATUS_CMD_OK; +} + static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_options, const woption *long_options, const wchar_t *cmd, int argc, wchar_t **argv, int *optind, parser_t &parser, @@ -538,38 +575,8 @@ static int argparse_parse_flags(argparse_cmd_opts_t &opts, const wchar_t *short_ assert(found != opts.options.end()); option_spec_t *opt_spec = found->second; - opt_spec->num_seen++; - if (opt_spec->num_allowed == 0) { - // It's a boolean flag. Save the flag we saw since it might be useful to know if the - // short or long flag was given. - if (long_idx == -1) { - opt_spec->vals.push_back(wcstring(1, L'-') + opt_spec->short_flag); - } else { - opt_spec->vals.push_back(L"--" + opt_spec->long_flag); - } - assert(!w.woptarg); - long_idx = -1; - continue; - } - - if (w.woptarg) { - int retval = validate_arg(opts, opt_spec, long_idx != -1, w.woptarg, streams); - if (retval != STATUS_CMD_OK) return retval; - } - - if (opt_spec->num_allowed == -1 || opt_spec->num_allowed == 1) { - // We're depending on `wgetopt_long()` to report that a mandatory value is missing if - // `opt_spec->num_allowed == 1` and thus return ':' so that we don't take this branch if - // the mandatory arg is missing. - opt_spec->vals.clear(); - if (w.woptarg) { - opt_spec->vals.push_back(w.woptarg); - } - } else { - assert(w.woptarg); - opt_spec->vals.push_back(w.woptarg); - } - + int retval = handle_flag(opts, opt_spec, long_idx, w.woptarg, streams); + if (retval != STATUS_CMD_OK) return retval; long_idx = -1; } @@ -607,7 +614,6 @@ static int argparse_parse_args(argparse_cmd_opts_t &opts, const wcstring_list_t retval = check_for_mutually_exclusive_flags(opts, streams); if (retval != STATUS_CMD_OK) return retval; - update_bool_flag_counts(opts); for (int i = optind; argv[i]; i++) opts.argv.push_back(argv[i]); diff --git a/tests/argparse.err b/tests/argparse.err index 37bcf54ca..fa8592ef3 100644 --- a/tests/argparse.err +++ b/tests/argparse.err @@ -29,6 +29,8 @@ argparse: Long flag 'short' already defined argparse: Implicit int flag '#' already defined # Defining an implicit int flag with modifiers argparse: Implicit int short flag 'v' does not allow modifiers like '=' +# Implicit int short flag only with custom validation fails +argparse: Value '499' for flag 'x' less than min allowed of '500' # Implicit int flag validation fails argparse: Value '765x' for flag 'max' is not an integer argparse: Value 'a1' for flag 'm' is not an integer diff --git a/tests/argparse.in b/tests/argparse.in index da5c5f1dc..6514d44b9 100644 --- a/tests/argparse.in +++ b/tests/argparse.in @@ -86,15 +86,42 @@ set -l for v in (set -l -n); set -e $v; end argparse 'v/verbose' '#-val' 't/token=' -- -123 a1 --token woohoo --234 -v a2 --verbose set -l + echo '# Should be set to 987' for v in (set -l -n); set -e $v; end argparse 'm#max' -- argle -987 bargle set -l + echo '# Should be set to 765' for v in (set -l -n); set -e $v; end argparse 'm#max' -- argle -987 bargle --max 765 set -l +echo '# Bool short flag only' +for v in (set -l -n); set -e $v; end +argparse 'C' 'v' -- -C -v arg1 -v arg2 +set -l + +echo '# Value taking short flag only' +for v in (set -l -n); set -e $v; end +argparse 'x=' 'v/verbose' -- --verbose arg1 -v -x arg2 +set -l + +echo '# Implicit int short flag only' +for v in (set -l -n); set -e $v; end +argparse 'x#' 'v/verbose' -- -v -v argle -v -x 321 bargle +set -l + +echo '# Implicit int short flag only with custom validation passes' +for v in (set -l -n); set -e $v; end +argparse 'x#!_validate_int --max 500' 'v/verbose' -- -v -v -x 499 -v +set -l + +echo '# Implicit int short flag only with custom validation fails' >&2 +for v in (set -l -n); set -e $v; end +argparse 'x#!_validate_int --min 500' 'v/verbose' -- -v -v -x 499 -v +set -l + ########## # Verify that flag value validation works. diff --git a/tests/argparse.out b/tests/argparse.out index eb3752ec8..81f19adee 100644 --- a/tests/argparse.out +++ b/tests/argparse.out @@ -38,6 +38,25 @@ argv 'argle' 'bargle' _flag_m 765 _flag_max 765 argv 'argle' 'bargle' +# Bool short flag only +_flag_C -C +_flag_v '-v' '-v' +argv 'arg1' 'arg2' +# Value taking short flag only +_flag_v '--verbose' '-v' +_flag_verbose '--verbose' '-v' +_flag_x arg2 +argv arg1 +# Implicit int short flag only +_flag_v '-v' '-v' '-v' +_flag_verbose '-v' '-v' '-v' +_flag_x 321 +argv 'argle' 'bargle' +# Implicit int short flag only with custom validation passes +_flag_v '-v' '-v' '-v' +_flag_verbose '-v' '-v' '-v' +_flag_x 499 +argv # Check the exit status from argparse validation _flag_name max _flag_value 83 From d068f846e893fe17d2a1d90d49a12275ebbcf976 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Fri, 21 Jul 2017 16:25:03 -0700 Subject: [PATCH 03/18] simplify `history` function The fix for #4232 allows us to simplify the `history` function slightly. --- share/functions/history.fish | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/share/functions/history.fish b/share/functions/history.fish index 0545aae23..bfe2651ad 100644 --- a/share/functions/history.fish +++ b/share/functions/history.fish @@ -48,12 +48,6 @@ function history --description "display or manipulate interactive command histor set show_time --show-time end - set -q _flag_null - and set -l null --null - - set -q _flag_case_sensitive - and set -l case_sensitive --case-sensitive - set -q _flag_prefix and set -l search_mode --prefix set -q _flag_contains @@ -96,9 +90,9 @@ function history --description "display or manipulate interactive command histor set -l pager less set -q PAGER and set pager $PAGER - builtin history search $search_mode $show_time $max_count $case_sensitive $null -- $argv | eval $pager + builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_null -- $argv | eval $pager else - builtin history search $search_mode $show_time $max_count $case_sensitive $null -- $argv + builtin history search $search_mode $show_time $max_count $_flag_case_sensitive $_flag_null -- $argv end case delete # interactively delete history @@ -112,14 +106,14 @@ function history --description "display or manipulate interactive command histor and set search_mode "--contains" if test $search_mode = "--exact" - builtin history delete $search_mode $case_sensitive $argv + builtin history delete $search_mode $_flag_case_sensitive $argv return end # TODO: Fix this so that requesting history entries with a timestamp works: # set -l found_items (builtin history search $search_mode $show_time -- $argv) set -l found_items - builtin history search $search_mode $case_sensitive --null -- $argv | while read -lz x + builtin history search $search_mode $_flag_case_sensitive --null -- $argv | while read -lz x set found_items $found_items $x end if set -q found_items[1] From 3fc0faaebb0e553d254723b118b24c72b7e10622 Mon Sep 17 00:00:00 2001 From: Raphael P Date: Fri, 21 Jul 2017 22:36:27 +0200 Subject: [PATCH 04/18] Update completions for heroku pg:backups heroku pgbackups has been deprecated and replaced by heroku pg:backups command. See: https://devcenter.heroku.com/changelog-items/623 Completion for new subcommand has been added as well. --- share/completions/heroku.fish | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/share/completions/heroku.fish b/share/completions/heroku.fish index 2d11b15bb..ae46b8ef5 100644 --- a/share/completions/heroku.fish +++ b/share/completions/heroku.fish @@ -74,7 +74,6 @@ complete $heroku_looking -xa maintenance -d 'manage maintenance mode for an app' complete $heroku_looking -xa members -d 'manage membership in organization accounts' complete $heroku_looking -xa orgs -d 'manage organization accounts' complete $heroku_looking -xa pg -d 'manage heroku-postgresql databases' -complete $heroku_looking -xa pgbackups -d 'manage backups of heroku postgresql databases' complete $heroku_looking -xa plugins -d 'manage plugins to the heroku gem' complete $heroku_looking -xa regions -d 'list available regions' complete $heroku_looking -xa stack -d 'manage the stack for an app' @@ -168,6 +167,19 @@ complete -c heroku -n '__fish_heroku_using_command logs' -s p -l ps -l PS -d "on complete -c heroku -n '__fish_heroku_using_command logs' -s s -l source -l SOURCE -d "only display logs from the given source" complete -c heroku -n '__fish_heroku_using_command logs' -s t -l tail -d "continually stream logs" +# PG subcommands +complete $heroku_looking -xa pg:backups -d "manage backups of heroku postgresql databases" +complete $heroku_looking -xa pg:backups:cancel -d "cancel an in-progress backup or restore (default newest)" +complete $heroku_looking -xa pg:backups:capture -d "capture a new backup" +complete $heroku_looking -xa pg:backups:delete -d "delete a backup" +complete $heroku_looking -xa pg:backups:download -d "downloads database backup" +complete $heroku_looking -xa pg:backups:info -d "get information about a specific backup" +complete $heroku_looking -xa pg:backups:restore -d "restore a backup (default latest) to a database" +complete $heroku_looking -xa pg:backups:schedule -d "schedule daily backups for given database" +complete $heroku_looking -xa pg:backups:schedules -d "list backup schedule" +complete $heroku_looking -xa pg:backups:unschedule -d "stop daily backups" +complete $heroku_looking -xa pg:backups:url -d "get secret but publicly accessible URL of a backup" + # PS subcommands complete $heroku_looking -xa ps:resize -d "resize dynos to the given size (DYNO1=1X|2X|PX)" complete -c heroku -n '__fish_heroku_using_command ps:resize' -fa '(__fish_list_heroku_dynos)' -d "resize dynos to the given size (DYNO1=1X|2X|PX)" From 7304d8241684f7890099cd8fd60ca045ded3d025 Mon Sep 17 00:00:00 2001 From: Fabian Homborg Date: Sat, 22 Jul 2017 14:09:07 +0200 Subject: [PATCH 05/18] Fix file completion for `tail` --- share/completions/tail.fish | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/share/completions/tail.fish b/share/completions/tail.fish index 66c426d66..72a6793c4 100644 --- a/share/completions/tail.fish +++ b/share/completions/tail.fish @@ -1,6 +1,6 @@ if tail --version > /dev/null ^ /dev/null complete -c tail -s c -l bytes -x -d 'output the last K bytes; alternatively, use -c +K to output bytes starting with the Kth of each file' - complete -c tail -s f -l follow -xa 'name descriptor' -d 'output appended data as the file grows; -f -l follow, and --follow=descriptor are equivalent' + complete -c tail -s f -l follow -a 'name descriptor' -d 'output appended data as the file grows; -f -l follow, and --follow=descriptor are equivalent' complete -c tail -s F -d 'same as --follow=name --retry' complete -c tail -s n -l lines -x -d 'output the last K lines, instead of the last 10; or use -n +K to output lines starting with the Kth' complete -c tail -l max-unchanged-stats -x -d 'with --follow=name, reopen a FILE which has not changed size after N iterations' @@ -9,8 +9,8 @@ if tail --version > /dev/null ^ /dev/null complete -c tail -l retry -d 'keep trying to open a file even when it is or becomes inaccessible; useful when following by name, i.e., with --follow=name' complete -c tail -s s -l sleep-interval -x -d 'with -f, sleep for approximately N seconds (default 1.0) between iterations' complete -c tail -s v -l verbose -d 'always output headers giving file names' - complete -c tail -l help -d 'display this help and exit' - complete -c tail -l version -d 'output version information and exit' + complete -c tail -x -l help -d 'display this help and exit' + complete -c tail -x -l version -d 'output version information and exit' else # OSX and similar - no longopts (and fewer shortopts) complete -c tail -s b -x -d 'output last K 512 byte blocks' complete -c tail -s c -x -d 'output the last K bytes or only K bytes with -r' From f8fa69f81766d77df48a68a66ec4d8d143469423 Mon Sep 17 00:00:00 2001 From: Rabah Meradi Date: Sat, 15 Jul 2017 13:32:00 +0200 Subject: [PATCH 06/18] Document how to erase a path from $PATH variable Fixes #3161 --- doc_src/tutorial.hdr | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc_src/tutorial.hdr b/doc_src/tutorial.hdr index 0e7727532..9d2ae535a 100644 --- a/doc_src/tutorial.hdr +++ b/doc_src/tutorial.hdr @@ -581,6 +581,12 @@ To prepend /usr/local/bin and /usr/sbin to `$PATH`, you can write: >_ set PATH /usr/local/bin /usr/sbin $PATH \endfish +To remove /usr/local/bin from `$PATH`, you can write: + +\fish{cli-dark} +>_ set PATH (string match -v /usr/local/bin $PATH) +\end{fish} + You can do so directly in `config.fish`, like you might do in other shells with `.profile`. See [this example](#path_example). A faster way is to modify the `$fish_user_paths` [universal variable](#tut_universal), which is automatically prepended to `$PATH`. For example, to permanently add `/usr/local/bin` to your `$PATH`, you could write: From d2d707a6faef50ad2b8cfe31ff49e33b6260944f Mon Sep 17 00:00:00 2001 From: Mohamed Akram Date: Fri, 21 Jul 2017 04:43:46 +0400 Subject: [PATCH 07/18] Ignore comments when creating man page completion --- share/tools/create_manpage_completions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/share/tools/create_manpage_completions.py b/share/tools/create_manpage_completions.py index fb63f8fab..0eb4045bc 100755 --- a/share/tools/create_manpage_completions.py +++ b/share/tools/create_manpage_completions.py @@ -528,7 +528,6 @@ class TypeDarwinManParser(ManParser): line = line.replace('.Nm', CMDNAME) line = line.replace('\\ ', ' ') line = line.replace('\& ', '') - line = line.replace(r'.\"', '') return line def is_option(self, line): @@ -567,6 +566,9 @@ class TypeDarwinManParser(ManParser): desc_lines = [] while lines and not self.is_option(lines[0]): line = lossy_unicode(lines.pop(0).strip()) + # Ignore comments + if line.startswith(r'.\"'): + continue if line.startswith('.'): line = self.groff_replace_escapes(line) line = self.trim_groff(line).strip() From 92f39f7b897fd7ea5289150ff2d2855bf98cfe91 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Tue, 25 Jul 2017 14:02:50 -0700 Subject: [PATCH 08/18] fix bug introduced by commit 86af63cd3 --- share/functions/funcsave.fish | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/share/functions/funcsave.fish b/share/functions/funcsave.fish index a52e1a3fe..870967958 100644 --- a/share/functions/funcsave.fish +++ b/share/functions/funcsave.fish @@ -22,16 +22,16 @@ function funcsave --description "Save the current definition of all specified fu end end - set -l res 0 + set -l retval 0 for funcname in $argv if functions -q -- $funcname - functions -- $i >$configdir/fish/functions/$funcname + functions -- $funcname >$configdir/fish/functions/$funcname.fish else printf (_ "%s: Unknown function '%s'\n") funcsave $funcname - set res 1 + set retval 1 end end - return $res + return $retval end From 8e2d1657564e2d06c6f977a3ed6a85d4af8115aa Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Mon, 24 Jul 2017 11:58:31 -0700 Subject: [PATCH 09/18] document need for double-quotes around `test` args --- doc_src/test.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc_src/test.txt b/doc_src/test.txt index 43d9f5e57..c5aa4b0fb 100644 --- a/doc_src/test.txt +++ b/doc_src/test.txt @@ -14,6 +14,8 @@ The first form (`test`) is preferred. For compatibility with other shells, the s This test is mostly POSIX-compatible. +When using a variable as an argument for a test operator you should almost always enclose it in double-quotes. There are only two situations it is safe to omit the quote marks. The first is when the argument is a literal string with no whitespace or other characters special to the shell (e.g., semicolon). For example, `test -b /my/file`. The second is using a variable that expands to exactly one element including if that element is the empty string (e.g., `set x ''`). If the variable is not set, set but with no value, or set to more than one value you must enclose it in double-quotes. For example, `test "$x" = "$y"`. Since it is always safe to enclose variables in double-quotes when used as `test` arguments that is the recommended practice. + \subsection test-files Operators for files and directories - `-b FILE` returns true if `FILE` is a block device. From a071deaf6176769088de5141e663c577108aa63c Mon Sep 17 00:00:00 2001 From: Thales Mello Date: Wed, 26 Jul 2017 08:31:35 -0300 Subject: [PATCH 10/18] Make npm run-script completion faster with `jq` (#4241) * Make npm run-script completion faster with `jq` When jq is available, it's actually faster to invoke jq and parse the `package.json` invoking the `npm` command. Also, prior to this commit, both `__fish_complete_npm` and `__fish_npm_run` were being run whenever completions for `npm run` subcommand was being used, which was actually making repetitive work (invoking npm command twice). This pull request is supposed to make completion without `jq` faster as well * Refactor npm.fish for code reutilization Created function to handle both cases of npm run completion parse, with or without `jq` completion. * Remove unecessary blank line --- share/completions/npm.fish | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/share/completions/npm.fish b/share/completions/npm.fish index d531dd1d3..0c57814a4 100644 --- a/share/completions/npm.fish +++ b/share/completions/npm.fish @@ -64,21 +64,28 @@ function __fish_complete_npm --description "Complete the commandline using npm's end # use npm completion for most of the things, -# except options completion because it sucks at it. +# except options completion (because it sucks at it) +# and run-script completion (reading package.json is faster). # see: https://github.com/npm/npm/issues/9524 # and: https://github.com/fish-shell/fish-shell/pull/2366 -complete -f -c npm -n 'not __fish_npm_needs_option' -a "(__fish_complete_npm)" +complete -f -c npm -n 'not __fish_npm_needs_option; and not __fish_npm_using_command run; and not __fish_npm_using_command run-script' -a "(__fish_complete_npm)" # list available npm scripts and their parial content +function __fish_parse_npm_run_completions + while read -l name + set -l trim 20 + read -l value + set value (string sub -l $trim -- $value) + printf "%s\t%s\n" $name $value + end +end + function __fish_npm_run # Like above, only try to call npm if there's a command by that name to facilitate aliases that call nvm. - if command -sq npm - command npm run | string match -r -v '^[^ ]|^$' | string trim | while read -l name - set -l trim 20 - read -l value - echo "$value" | cut -c1-$trim | read -l value - printf "%s\t%s\n" $name $value - end + if command -sq jq; and test -e package.json + jq -r '.scripts | to_entries[] | .key,.value' Date: Wed, 26 Jul 2017 20:41:00 -0700 Subject: [PATCH 11/18] fix ssh config `Include` file handling Fixes #4253 --- share/functions/__fish_print_hostnames.fish | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/share/functions/__fish_print_hostnames.fish b/share/functions/__fish_print_hostnames.fish index 8844154d2..0bf89e334 100644 --- a/share/functions/__fish_print_hostnames.fish +++ b/share/functions/__fish_print_hostnames.fish @@ -44,14 +44,14 @@ function __fish_print_hostnames -d "Print a list of known hostnames" function _ssh_include --argument-names ssh_config # Relative paths in Include directive use /etc/ssh or ~/.ssh depending on # system or user level config. -F will not override this behaviour - if test $ssh_config = '/etc/ssh/ssh_config' + set -l relative_path $HOME/.ssh + if string match '/etc/ssh/*' -- $ssh_config set relative_path '/etc/ssh' - else - set relative_path $HOME/.ssh end function _recursive --no-scope-shadowing - set paths + set -l orig_dir $PWD + set -l paths for config in $argv set paths $paths (cat $config ^/dev/null \ # Keep only Include lines @@ -62,10 +62,11 @@ function __fish_print_hostnames -d "Print a list of known hostnames" | string trim | string replace -r -a '\s+' ' ') end + cd $relative_path set -l new_paths for path in $paths set -l expanded_path - eval set expanded_path (echo $path) + eval "set expanded_path (printf \"%s\n\" $path)" for path in $expanded_path # Resolve "relative" paths in accordance to ssh path resolution if string match -qv '/*' $path @@ -75,9 +76,9 @@ function __fish_print_hostnames -d "Print a list of known hostnames" set new_paths $new_paths $path end end + cd $orig_dir if test -n "$new_paths" - _recursive $new_paths end end From 4e026588f4aaf26fc49c16657359497d90ecce38 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 27 Jul 2017 14:34:19 -0700 Subject: [PATCH 12/18] modify prev commit to use builtin cd Using the `cd` function can have undesirable side effects like mucking with the directory history. So force the use of the builtin cd. --- share/functions/__fish_print_hostnames.fish | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/share/functions/__fish_print_hostnames.fish b/share/functions/__fish_print_hostnames.fish index 0bf89e334..efa3cac4f 100644 --- a/share/functions/__fish_print_hostnames.fish +++ b/share/functions/__fish_print_hostnames.fish @@ -62,7 +62,7 @@ function __fish_print_hostnames -d "Print a list of known hostnames" | string trim | string replace -r -a '\s+' ' ') end - cd $relative_path + builtin cd $relative_path set -l new_paths for path in $paths set -l expanded_path @@ -76,7 +76,7 @@ function __fish_print_hostnames -d "Print a list of known hostnames" set new_paths $new_paths $path end end - cd $orig_dir + builtin cd $orig_dir if test -n "$new_paths" _recursive $new_paths From 18219646a085d3e536e1af6a28471283813eb04a Mon Sep 17 00:00:00 2001 From: Jon Eyolfson Date: Thu, 27 Jul 2017 19:49:03 -0400 Subject: [PATCH 13/18] Remove completer_t::complete_special_cd The class `completer_t` declares `complete_special_cd`, an unused method. I searched the entire source tree and this declaration seems to be the only instance of `complete_special_cd`. There is no definition or uses which likely means this is dead code. --- src/complete.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/complete.cpp b/src/complete.cpp index 3ca2c8131..ac364724c 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -317,8 +317,6 @@ class completer_t { void complete_param_expand(const wcstring &str, bool do_file, bool handle_as_special_cd = false); - void complete_special_cd(const wcstring &str); - void complete_cmd(const wcstring &str, bool use_function, bool use_builtin, bool use_command, bool use_implicit_cd); From a1c4475c0def343793131dfd7a75ec85483d4c21 Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 25 Jul 2017 23:26:23 -0500 Subject: [PATCH 14/18] Silenced (wrong) -Wmaybe-uninitialized warnings --- src/expand.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/expand.cpp b/src/expand.cpp index 029c0af76..e00a1bff0 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -492,7 +492,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, while (const job_t *j = jobs.next()) { if (j->command_is_empty()) continue; - size_t offset; + size_t offset = 0; if (match_pid(j->command(), proc, &offset)) { if (flags & EXPAND_FOR_COMPLETIONS) { append_completion(completions, j->command_wcstr() + offset + wcslen(proc), @@ -514,7 +514,7 @@ static bool find_job(const wchar_t *proc, expand_flags_t flags, for (const process_ptr_t &p : j->processes) { if (p->actual_cmd.empty()) continue; - size_t offset; + size_t offset = 0; if (match_pid(p->actual_cmd, proc, &offset)) { if (flags & EXPAND_FOR_COMPLETIONS) { append_completion(completions, wcstring(p->actual_cmd, offset + wcslen(proc)), @@ -552,7 +552,7 @@ static void find_process(const wchar_t *proc, expand_flags_t flags, pid_t process_pid; process_iterator_t iterator; while (iterator.next_process(&process_name, &process_pid)) { - size_t offset; + size_t offset = 0; if (match_pid(process_name, proc, &offset)) { if (flags & EXPAND_FOR_COMPLETIONS) { append_completion(out, process_name.c_str() + offset + wcslen(proc), From 7a18c37b3959207f516718c7c991271955240eec Mon Sep 17 00:00:00 2001 From: Mahmoud Al-Qudsi Date: Tue, 25 Jul 2017 23:34:22 -0500 Subject: [PATCH 15/18] Using write_ignore instead of write where the result is not checked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This silences warnings from the compiler about ignoring return value of ‘ssize_t write(int, const void*, size_t)’, declared with attribute warn_unused_result [-Wunused-result]. --- src/common.cpp | 16 +++------------- src/common.h | 6 +----- src/env_universal_common.cpp | 4 ++-- src/path.cpp | 2 +- src/reader.cpp | 8 ++++---- src/wutil.cpp | 2 +- 6 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/common.cpp b/src/common.cpp index d0dea1c20..b52dbf076 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -598,16 +598,6 @@ void __attribute__((noinline)) debug(int level, const char *msg, ...) { errno = errno_old; } -void read_ignore(int fd, void *buff, size_t count) { - size_t ignore __attribute__((unused)); - ignore = read(fd, buff, count); -} - -void write_ignore(int fd, const void *buff, size_t count) { - size_t ignore __attribute__((unused)); - ignore = write(fd, buff, count); -} - void debug_safe(int level, const char *msg, const char *param1, const char *param2, const char *param3, const char *param4, const char *param5, const char *param6, const char *param7, const char *param8, const char *param9, const char *param10, @@ -626,14 +616,14 @@ void debug_safe(int level, const char *msg, const char *param1, const char *para const char *end = strchr(cursor, '%'); if (end == NULL) end = cursor + strlen(cursor); - write_ignore(STDERR_FILENO, cursor, end - cursor); + (void)write(STDERR_FILENO, cursor, end - cursor); if (end[0] == '%' && end[1] == 's') { // Handle a format string. assert(param_idx < sizeof params / sizeof *params); const char *format = params[param_idx++]; if (!format) format = "(null)"; - write_ignore(STDERR_FILENO, format, strlen(format)); + (void)write(STDERR_FILENO, format, strlen(format)); cursor = end + 2; } else if (end[0] == '\0') { // Must be at the end of the string. @@ -645,7 +635,7 @@ void debug_safe(int level, const char *msg, const char *param1, const char *para } // We always append a newline. - write_ignore(STDERR_FILENO, "\n", 1); + (void)write(STDERR_FILENO, "\n", 1); errno = errno_old; } diff --git a/src/common.h b/src/common.h index d4a2f646f..aa0085378 100644 --- a/src/common.h +++ b/src/common.h @@ -179,10 +179,6 @@ extern bool g_profiling_active; /// Name of the current program. Should be set at startup. Used by the debug function. extern const wchar_t *program_name; -// Variants of read() and write() that ignores return values, defeating a warning. -void read_ignore(int fd, void *buff, size_t count); -void write_ignore(int fd, const void *buff, size_t count); - /// Set to false at run-time if it's been determined we can't trust the last modified timestamp on /// the tty. extern bool has_working_tty_timestamps; @@ -207,7 +203,7 @@ extern bool has_working_tty_timestamps; { \ char exit_read_buff; \ show_stackframe(L'E'); \ - read_ignore(0, &exit_read_buff, 1); \ + (void)read(0, &exit_read_buff, 1); \ exit_without_destructors(1); \ } diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index c340e5961..00228113f 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -1209,7 +1209,7 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { // would cause us to hang! size_t read_amt = 64 * 1024; void *buff = malloc(read_amt); - read_ignore(this->pipe_fd, buff, read_amt); + (void)read(this->pipe_fd, buff, read_amt); free(buff); } @@ -1308,7 +1308,7 @@ class universal_notifier_named_pipe_t : public universal_notifier_t { while (this->readback_amount > 0) { char buff[64]; size_t amt_to_read = mini(this->readback_amount, sizeof buff); - read_ignore(this->pipe_fd, buff, amt_to_read); + (void)read(this->pipe_fd, buff, amt_to_read); this->readback_amount -= amt_to_read; } assert(this->readback_amount == 0); diff --git a/src/path.cpp b/src/path.cpp index fcb383474..7af742974 100644 --- a/src/path.cpp +++ b/src/path.cpp @@ -275,7 +275,7 @@ static void maybe_issue_path_warning(const wcstring &which_dir, const wcstring & debug(0, _(L"The error was '%s'."), strerror(saved_errno)); debug(0, _(L"Please set $%ls to a directory where you have write access."), env_var); } - write(STDERR_FILENO, "\n", 1); + (void)write(STDERR_FILENO, "\n", 1); } static void path_create(wcstring &path, const wcstring &xdg_var, const wcstring &which_dir, diff --git a/src/reader.cpp b/src/reader.cpp index 1e0e05698..8dff6cc6a 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -696,14 +696,14 @@ void reader_write_title(const wcstring &cmd, bool reset_cursor_position) { for (size_t i = 0; i < lst.size(); i++) { fputws(lst.at(i).c_str(), stdout); } - write(STDOUT_FILENO, "\a", 1); + (void)write(STDOUT_FILENO, "\a", 1); } proc_pop_interactive(); set_color(rgb_color_t::reset(), rgb_color_t::reset()); if (reset_cursor_position && !lst.empty()) { // Put the cursor back at the beginning of the line (issue #2453). - write(STDOUT_FILENO, "\r", 1); + (void)write(STDOUT_FILENO, "\r", 1); } } @@ -1291,7 +1291,7 @@ static void reader_flash() { } reader_repaint(); - write(STDOUT_FILENO, "\a", 1); + (void)write(STDOUT_FILENO, "\a", 1); pollint.tv_sec = 0; pollint.tv_nsec = 100 * 1000000; @@ -3229,7 +3229,7 @@ const wchar_t *reader_readline(int nchars) { reader_repaint_if_needed(); } - write(STDOUT_FILENO, "\n", 1); + (void)write(STDOUT_FILENO, "\n", 1); // Ensure we have no pager contents when we exit. if (!data->pager.empty()) { diff --git a/src/wutil.cpp b/src/wutil.cpp index 5b69c47f4..fd1586950 100644 --- a/src/wutil.cpp +++ b/src/wutil.cpp @@ -338,7 +338,7 @@ void safe_perror(const char *message) { safe_append(buff, safe_strerror(err), sizeof buff); safe_append(buff, "\n", sizeof buff); - write_ignore(STDERR_FILENO, buff, strlen(buff)); + (void)write(STDERR_FILENO, buff, strlen(buff)); errno = err; } From 96fca8b4ecfc9fa097a20e7f6396ecbaba21fbf8 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Thu, 27 Jul 2017 21:32:49 -0700 Subject: [PATCH 16/18] fix how fish behaves when FISH_HISTORY is set Without this change setting `FISH_HISTORY` causes interactive input to no longer provide autosuggestions, completions, etc. Fixes #4234 --- src/env.cpp | 3 +-- src/reader.cpp | 5 +++++ src/reader.h | 3 +++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/env.cpp b/src/env.cpp index 2a4cf434a..eec86d2ec 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -613,8 +613,7 @@ static void react_to_variable_change(const wcstring &key) { } else if (key == L"FISH_READ_BYTE_LIMIT") { env_set_read_limit(); } else if (key == L"FISH_HISTORY") { - history_destroy(); - reader_push(history_session_id().c_str()); + reader_change_history(history_session_id().c_str()); } } diff --git a/src/reader.cpp b/src/reader.cpp index 8dff6cc6a..c46d13af8 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -1995,6 +1995,11 @@ static parser_test_error_bits_t default_test(const wchar_t *b) { return 0; } +void reader_change_history(const wchar_t *name) { + data->history->save(); + data->history = &history_t::history_with_name(name); +} + void reader_push(const wchar_t *name) { reader_data_t *n = new reader_data_t(); diff --git a/src/reader.h b/src/reader.h index c773ea1ed..3dc0d0802 100644 --- a/src/reader.h +++ b/src/reader.h @@ -72,6 +72,9 @@ const wchar_t *reader_current_filename(); /// \param fn The fileanme to push void reader_push_current_filename(const wchar_t *fn); +/// Change the history file for the current command reading context. +void reader_change_history(const wchar_t *fn); + /// Pop the current filename from the stack of read files. void reader_pop_current_filename(); From d68b631919e4a4a0c147d54d989b0bb8fbf1adf5 Mon Sep 17 00:00:00 2001 From: Daniel K Date: Fri, 28 Jul 2017 19:59:00 +0200 Subject: [PATCH 17/18] fix check for existing variable --- share/completions/git.fish | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/share/completions/git.fish b/share/completions/git.fish index 6a2b92746..d8e9df6d7 100644 --- a/share/completions/git.fish +++ b/share/completions/git.fish @@ -161,7 +161,7 @@ function __fish_git_using_command # Check aliases. set -l varname __fish_git_alias_(string escape --style=var -- $cmd) - set -q $$varname + set -q $varname and contains -- $$varname $argv and return 0 return 1 From f4414a06311267de8940a88fb18a728d2181e8f8 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sat, 29 Jul 2017 21:50:39 -0700 Subject: [PATCH 18/18] implement `complete -k` as a no-op Fixes #4270 --- src/builtin_complete.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/builtin_complete.cpp b/src/builtin_complete.cpp index c05c532fa..8ab13214d 100644 --- a/src/builtin_complete.cpp +++ b/src/builtin_complete.cpp @@ -129,7 +129,7 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wcstring_list_t path; wcstring_list_t wrap_targets; - static const wchar_t *short_options = L":a:c:p:s:l:o:d:frxeuAn:C::w:h"; + static const wchar_t *short_options = L":a:c:kp:s:l:o:d:frxeuAn:C::w:h"; static const struct woption long_options[] = {{L"exclusive", no_argument, NULL, 'x'}, {L"no-files", no_argument, NULL, 'f'}, {L"require-parameter", no_argument, NULL, 'r'}, @@ -147,6 +147,7 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { {L"wraps", required_argument, NULL, 'w'}, {L"do-complete", optional_argument, NULL, 'C'}, {L"help", no_argument, NULL, 'h'}, + {L"keep-order", no_argument, NULL, 'k'}, {NULL, 0, NULL, 0}}; int opt; @@ -165,6 +166,11 @@ int builtin_complete(parser_t &parser, io_streams_t &streams, wchar_t **argv) { result_mode |= NO_COMMON; break; } + case 'k': { + // This is a no-op in fish 2.7. It is implemented in fish 3.0. We want it to be + // silently ignored if someone happens to use a completion that uses this flag. + break; + } case 'p': case 'c': { wcstring tmp;