From 6b1c939b67e85c5617f1a1667d6c2e268b984f89 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Sun, 30 Apr 2017 19:53:13 -0700 Subject: [PATCH] rename --metadata to --details Discussion in issue #3295 resulted in a decisions to rename the functions --metadata flag to --details. This also fixes a bug in the definition of the short flags for the `functions` command. The `-e` flag does not take an argument and therefore should not be defined as `e:`. Notice that the long form, `--erase`, specifies `no_argument`. This discrepency happened to work due to a quirk of how the flag parsing loop was written. --- CHANGELOG.md | 2 +- doc_src/functions.txt | 4 +-- src/builtin.cpp | 79 +++++++++++++++---------------------------- tests/functions.in | 38 ++++++++++----------- 4 files changed, 50 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f644eae1f..ba94c0a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ - Fish is now more forgiving of missing or invalid $TERM values (#3850). - The `string` command now supports a `repeat` subcommand with the obvious behavior (#3864). - The `string match` command now supports a `--filter` flag to emit the entire string partially matched by a pattern (#3957). -- The `functions --metadata --verbose` output now includes the function description (#597). +- The `functions --details --verbose` output now includes the function description (#597). - Completions for `helm` added (#3829). - Empty components in $CDPATH, $MANPATH and $PATH are now converted to "." (#2106, #3914). - `type` now no longer requires `which`, which means it is no longer a dependency (#3912, #3945). diff --git a/doc_src/functions.txt b/doc_src/functions.txt index 9aae7e069..042c88977 100644 --- a/doc_src/functions.txt +++ b/doc_src/functions.txt @@ -3,7 +3,7 @@ \subsection functions-synopsis Synopsis \fish{synopsis} functions [ -a | --all ] [ -n | --names ] -functions [ -m | --metadata ] [ -v ] FUNCTION +functions [ -D | --details ] [ -v ] FUNCTION functions -c OLDNAME NEWNAME functions -d DESCRIPTION FUNCTION functions [ -e | -q ] FUNCTIONS... @@ -23,7 +23,7 @@ The following options are available: - `-e` or `--erase` causes the specified functions to be erased. -- `-m` or `--metadata` reports the path name where each function is defined or could be autoloaded, `stdin` if the function was defined interactively or on the command line or by reading stdin, and `n/a` if the function isn't available. If the `--verbose` option is also specified then five lines are written: +- `-D` or `--details` reports the path name where each function is defined or could be autoloaded, `stdin` if the function was defined interactively or on the command line or by reading stdin, and `n/a` if the function isn't available. If the `--verbose` option is also specified then five lines are written: -# the pathname as already described, -# `autoloaded`, `not-autoloaded` or `n/a`, diff --git a/src/builtin.cpp b/src/builtin.cpp index 92d6afdcf..d4199ae9c 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -1084,50 +1084,38 @@ static int report_function_metadata(const wchar_t *funcname, bool verbose, io_st /// The functions builtin, used for listing and erasing functions. static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t **argv) { - wgetopter_t w; - int i; - int erase = 0; - wchar_t *desc = 0; - + wchar_t *desc = NULL; int argc = builtin_count_args(argv); - int list = 0; - int show_hidden = 0; - int res = STATUS_BUILTIN_OK; - int query = 0; - int copy = 0; + bool erase = false; + bool list = false; + bool show_hidden = false; + bool query = false; + bool copy = false; bool report_metadata = false; bool verbose = false; + int res = STATUS_BUILTIN_OK; + static const wchar_t *short_options = L"Dacehnqv"; static const struct woption long_options[] = { {L"erase", no_argument, NULL, 'e'}, {L"description", required_argument, NULL, 'd'}, {L"names", no_argument, NULL, 'n'}, {L"all", no_argument, NULL, 'a'}, {L"help", no_argument, NULL, 'h'}, {L"query", no_argument, NULL, 'q'}, - {L"copy", no_argument, NULL, 'c'}, {L"metadata", no_argument, NULL, 'm'}, + {L"copy", no_argument, NULL, 'c'}, {L"details", no_argument, NULL, 'D'}, {L"verbose", no_argument, NULL, 'v'}, {NULL, 0, NULL, 0}}; - while (1) { - int opt_index = 0; - - int opt = w.wgetopt_long(argc, argv, L"ed:mnahqcv", long_options, &opt_index); - if (opt == -1) break; - + int opt; + wgetopter_t w; + while ((opt = w.wgetopt_long(argc, argv, short_options, long_options, NULL)) != -1) { switch (opt) { - case 0: { - if (long_options[opt_index].flag != 0) break; - streams.err.append_format(BUILTIN_ERR_UNKNOWN, argv[0], - long_options[opt_index].name); - builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; - } case 'v': { verbose = true; break; } case 'e': { - erase = 1; + erase = true; break; } - case 'm': { + case 'D': { report_metadata = true; break; } @@ -1136,11 +1124,11 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** break; } case 'n': { - list = 1; + list = true; break; } case 'a': { - show_hidden = 1; + show_hidden = true; break; } case 'h': { @@ -1148,11 +1136,11 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** return STATUS_BUILTIN_OK; } case 'q': { - query = 1; + query = true; break; } case 'c': { - copy = 1; + copy = true; break; } case '?': { @@ -1170,15 +1158,12 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** int describe = desc ? 1 : 0; if (erase + describe + list + query + copy > 1) { streams.err.append_format(_(L"%ls: Invalid combination of options\n"), argv[0]); - builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } if (erase) { - int i; - for (i = w.woptind; i < argc; i++) function_remove(argv[i]); + for (int i = w.woptind; i < argc; i++) function_remove(argv[i]); return STATUS_BUILTIN_OK; } else if (desc) { wchar_t *func; @@ -1186,15 +1171,13 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** if (argc - w.woptind != 1) { streams.err.append_format(_(L"%ls: Expected exactly one function name\n"), argv[0]); builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } + func = argv[w.woptind]; if (!function_exists(func)) { streams.err.append_format(_(L"%ls: Function '%ls' does not exist\n"), argv[0], func); - builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } @@ -1202,29 +1185,26 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** return STATUS_BUILTIN_OK; } else if (report_metadata) { if (argc - w.woptind != 1) { - streams.err.append_format( - _(L"%ls: Expected exactly one function name for --metadata\n"), argv[0]); + streams.err.append_format(_(L"%ls: Expected exactly one function name for --details\n"), + argv[0]); return STATUS_BUILTIN_ERROR; } const wchar_t *funcname = argv[w.woptind]; return report_function_metadata(funcname, verbose, streams, false); } else if (list || (argc == w.woptind)) { - int is_screen = !streams.out_is_redirected && isatty(STDOUT_FILENO); - size_t i; wcstring_list_t names = function_get_names(show_hidden); std::sort(names.begin(), names.end()); + bool is_screen = !streams.out_is_redirected && isatty(STDOUT_FILENO); if (is_screen) { wcstring buff; - - for (i = 0; i < names.size(); i++) { + for (size_t i = 0; i < names.size(); i++) { buff.append(names.at(i)); buff.append(L", "); } - streams.out.append(reformat_for_screen(buff)); } else { - for (i = 0; i < names.size(); i++) { + for (size_t i = 0; i < names.size(); i++) { streams.out.append(names.at(i).c_str()); streams.out.append(L"\n"); } @@ -1240,7 +1220,6 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** L"and new function name)\n"), argv[0]); builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } current_func = argv[w.woptind]; @@ -1250,7 +1229,6 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** streams.err.append_format(_(L"%ls: Function '%ls' does not exist\n"), argv[0], current_func.c_str()); builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } @@ -1267,7 +1245,6 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** _(L"%ls: Function '%ls' already exists. Cannot create copy '%ls'\n"), argv[0], new_func.c_str(), current_func.c_str()); builtin_print_help(parser, streams, argv[0], streams.err); - return STATUS_BUILTIN_ERROR; } @@ -1275,10 +1252,10 @@ static int builtin_functions(parser_t &parser, io_streams_t &streams, wchar_t ** return STATUS_BUILTIN_ERROR; } - for (i = w.woptind; i < argc; i++) { - if (!function_exists(argv[i])) + for (int i = w.woptind; i < argc; i++) { + if (!function_exists(argv[i])) { res++; - else { + } else { if (!query) { if (i != w.woptind) streams.out.append(L"\n"); const wchar_t *funcname = argv[w.woptind]; diff --git a/tests/functions.in b/tests/functions.in index dbe934eba..4e2f03433 100644 --- a/tests/functions.in +++ b/tests/functions.in @@ -6,57 +6,57 @@ function f1 end # ========== -# Verify that `functions --metadata` works as expected when given too many args. -set x (functions --metadata f1 f2 2>&1) -if test "$x" != "functions: Expected exactly one function name for --metadata" - echo "Unexpected output for 'functions --metadata f1 f2': $x" >&2 +# Verify that `functions --details` works as expected when given too many args. +set x (functions --details f1 f2 2>&1) +if test "$x" != "functions: Expected exactly one function name for --details" + echo "Unexpected output for 'functions --details f1 f2': $x" >&2 end # ========== -# Verify that `functions --metadata` works as expected when given the name of a +# Verify that `functions --details` works as expected when given the name of a # known function. -set x (functions --metadata f1) +set x (functions --details f1) if test "$x" != "stdin" - echo "Unexpected output for 'functions --metadata f1': $x" >&2 + echo "Unexpected output for 'functions --details f1': $x" >&2 end # ========== -# Verify that `functions --metadata` works as expected when given the name of an +# Verify that `functions --details` works as expected when given the name of an # unknown function. -set x (functions -m f2) +set x (functions -D f2) if test "$x" != "n/a" - echo "Unexpected output for 'functions --metadata f2': $x" >&2 + echo "Unexpected output for 'functions --details f2': $x" >&2 end # ========== -# Verify that `functions --metadata` works as expected when given the name of a +# Verify that `functions --details` works as expected when given the name of a # function that could be autoloaded but isn't currently loaded. -set x (functions -m abbr) +set x (functions -D abbr) if test (count $x) -ne 1 or not string match -q '*/share/functions/abbr.fish' "$x" - echo "Unexpected output for 'functions -m abbr': $x" >&2 + echo "Unexpected output for 'functions -D abbr': $x" >&2 end # ========== -# Verify that `functions --verbose --metadata` works as expected when given the name of a +# Verify that `functions --verbose --details` works as expected when given the name of a # function that was autoloaded. -set x (functions -v -m abbr) +set x (functions -v -D abbr) if test (count $x) -ne 5 or not string match -q '*/share/functions/abbr.fish' $x[1] or test $x[2] != autoloaded or test $x[3] != 1 or test $x[4] != scope-shadowing or test $x[5] != 'Manage abbreviations' - echo "Unexpected output for 'functions -v -m abbr': $x" >&2 + echo "Unexpected output for 'functions -v -D abbr': $x" >&2 end # ========== -# Verify that `functions --verbose --metadata` properly escapes a function +# Verify that `functions --verbose --details` properly escapes a function # with a multiline description. function multiline_descr -d 'line 1\n line 2 & more; way more' end -set x (functions -v -m multiline_descr) +set x (functions -v -D multiline_descr) if test $x[5] != 'line 1\\\\n\\nline 2 & more; way more' - echo "Unexpected output for 'functions -v -m multiline_descr': $x" >&2 + echo "Unexpected output for 'functions -v -D multiline_descr': $x" >&2 end