diff options
| -rw-r--r-- | builtin.cpp | 95 | ||||
| -rw-r--r-- | builtin_commandline.cpp | 9 | ||||
| -rw-r--r-- | common.cpp | 9 | ||||
| -rw-r--r-- | common.h | 3 | ||||
| -rw-r--r-- | doc_src/function.txt | 2 | ||||
| -rw-r--r-- | fish.xcodeproj/project.pbxproj | 6 | ||||
| -rw-r--r-- | osx/config.h | 4 | ||||
| -rw-r--r-- | tests/function.in | 14 | ||||
| -rw-r--r-- | tests/function.out | 4 |
9 files changed, 97 insertions, 49 deletions
diff --git a/builtin.cpp b/builtin.cpp index 71ba8a32..59778862 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -1655,7 +1655,7 @@ static int builtin_functions(parser_t &parser, wchar_t **argv) return STATUS_BUILTIN_ERROR; } - if ((wcsfuncname(new_func.c_str()) != 0) || parser_keywords_is_reserved(new_func)) + if ((wcsfuncname(new_func) != 0) || parser_keywords_is_reserved(new_func)) { append_format(stderr_buffer, _(L"%ls: Illegal function name '%ls'\n"), @@ -2002,16 +2002,24 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr int res=STATUS_BUILTIN_OK; wchar_t *desc=0; std::vector<event_t> events; - std::auto_ptr<wcstring_list_t> named_arguments(NULL); + + bool has_named_arguments = false; + wcstring_list_t named_arguments; wcstring_list_t inherit_vars; - wchar_t *name = 0; bool shadows = true; woptind=0; wcstring_list_t wrap_targets; - + + /* If -a/--argument-names is specified before the function name, + then the function name is the last positional, e.g. `function -a arg1 arg2 name`. + If it is specified after the function name (or not specified at all) then the + function name is the first positional. This is the common case. */ + bool name_is_first_positional = true; + wcstring_list_t positionals; + const struct woption long_options[] = { { L"description", required_argument, 0, 'd' }, @@ -2032,9 +2040,10 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr { int opt_index = 0; + // The leading - here specifies RETURN_IN_ORDER int opt = wgetopt_long(argc, argv, - L"d:s:j:p:v:e:haSV:", + L"-d:s:j:p:v:e:haSV:", long_options, &opt_index); if (opt == -1) @@ -2176,8 +2185,9 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr } case 'a': - if (named_arguments.get() == NULL) - named_arguments.reset(new wcstring_list_t); + has_named_arguments = true; + /* The function name is the first positional unless -a comes before all positionals */ + name_is_first_positional = ! positionals.empty(); break; case 'S': @@ -2204,6 +2214,11 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr case 'h': builtin_print_help(parser, argv[0], stdout_buffer); return STATUS_BUILTIN_OK; + + case 1: + assert(woptarg != NULL); + positionals.push_back(woptarg); + break; case '?': builtin_unknown_option(parser, argv[0], argv[woptind-1]); @@ -2216,87 +2231,93 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr if (!res) { - - if (argc == woptind) + /* Determine the function name, and remove it from the list of positionals */ + wcstring function_name; + bool name_is_missing = positionals.empty(); + if (! name_is_missing) + { + if (name_is_first_positional) + { + function_name = positionals.front(); + positionals.erase(positionals.begin()); + } + else + { + function_name = positionals.back(); + positionals.erase(positionals.end() - 1); + } + } + + if (name_is_missing) { append_format(*out_err, _(L"%ls: Expected function name\n"), argv[0]); res=1; } - else if (wcsfuncname(argv[woptind])) + else if (wcsfuncname(function_name)) { append_format(*out_err, _(L"%ls: Illegal function name '%ls'\n"), argv[0], - argv[woptind]); + function_name.c_str()); res=1; } - else if (parser_keywords_is_reserved(argv[woptind])) + else if (parser_keywords_is_reserved(function_name)) { append_format(*out_err, _(L"%ls: The name '%ls' is reserved,\nand can not be used as a function name\n"), argv[0], - argv[woptind]); + function_name.c_str()); res=1; } - else if (! wcslen(argv[woptind])) + else if (function_name.empty()) { append_format(*out_err, _(L"%ls: No function name given\n"), argv[0]); res=1; } else { - - name = argv[woptind++]; - - if (named_arguments.get()) + if (has_named_arguments) { - while (woptind < argc) + /* All remaining positionals are named arguments */ + named_arguments.swap(positionals); + for (size_t i=0; i < named_arguments.size(); i++) { - if (wcsvarname(argv[woptind])) + if (wcsvarname(named_arguments.at(i))) { append_format(*out_err, _(L"%ls: Invalid variable name '%ls'\n"), argv[0], - argv[woptind]); + named_arguments.at(i).c_str()); res = STATUS_BUILTIN_ERROR; break; } - - named_arguments->push_back(argv[woptind++]); } } - else if (woptind != argc) + else if (! positionals.empty()) { + // +1 because we already got the function name append_format(*out_err, _(L"%ls: Expected one argument, got %d\n"), argv[0], - argc); + positionals.size() + 1); res=1; - } } - } - if (res) - { - builtin_print_help(parser, argv[0], *out_err); - } - else - { + /* Here we actually define the function! */ function_data_t d; - d.name = name; + d.name = function_name; if (desc) d.description = desc; d.events.swap(events); d.shadows = shadows; - if (named_arguments.get()) - d.named_arguments.swap(*named_arguments); + d.named_arguments.swap(named_arguments); d.inherit_vars.swap(inherit_vars); for (size_t i=0; i<d.events.size(); i++) @@ -2312,7 +2333,7 @@ int define_function(parser_t &parser, const wcstring_list_t &c_args, const wcstr // Handle wrap targets for (size_t w=0; w < wrap_targets.size(); w++) { - complete_add_wrapper(name, wrap_targets.at(w)); + complete_add_wrapper(function_name, wrap_targets.at(w)); } } diff --git a/builtin_commandline.cpp b/builtin_commandline.cpp index 339cb0d1..01bc123a 100644 --- a/builtin_commandline.cpp +++ b/builtin_commandline.cpp @@ -206,7 +206,9 @@ static void write_part(const wchar_t *begin, { case TOK_STRING: { - out.append(escape_string(tok_last(&tok), UNESCAPE_INCOMPLETE)); + wcstring tmp = tok_last(&tok); + unescape_string_in_place(&tmp, UNESCAPE_INCOMPLETE); + out.append(tmp); out.push_back(L'\n'); break; } @@ -230,8 +232,9 @@ static void write_part(const wchar_t *begin, } // debug( 0, L"woot2 %ls -> %ls", buff, esc ); - - stdout_buffer.append(begin, end - begin); + wcstring tmp = wcstring(begin, end - begin); + unescape_string_in_place(&tmp, UNESCAPE_INCOMPLETE); + stdout_buffer.append(tmp); stdout_buffer.append(L"\n"); } @@ -481,9 +481,14 @@ const wchar_t *wcsvarname(const wchar_t *str) return NULL; } -const wchar_t *wcsfuncname(const wchar_t *str) +const wchar_t *wcsvarname(const wcstring &str) { - return wcschr(str, L'/'); + return wcsvarname(str.c_str()); +} + +const wchar_t *wcsfuncname(const wcstring &str) +{ + return wcschr(str.c_str(), L'/'); } @@ -696,6 +696,7 @@ void append_formatv(wcstring &str, const wchar_t *format, va_list ap); */ const wchar_t *wcsvarname(const wchar_t *str); +const wchar_t *wcsvarname(const wcstring &str); /** @@ -704,7 +705,7 @@ const wchar_t *wcsvarname(const wchar_t *str); \return null if this is a valid name, and a pointer to the first invalid character otherwise */ -const wchar_t *wcsfuncname(const wchar_t *str); +const wchar_t *wcsfuncname(const wcstring &str); /** Test if the given string is valid in a variable name diff --git a/doc_src/function.txt b/doc_src/function.txt index 1f9fb47d..e58d6513 100644 --- a/doc_src/function.txt +++ b/doc_src/function.txt @@ -2,7 +2,7 @@ \subsection function-synopsis Synopsis \fish{synopsis} -function [OPTIONS] NAME; BODY; end +function NAME [OPTIONS]; BODY; end \endfish \subsection function-description Description diff --git a/fish.xcodeproj/project.pbxproj b/fish.xcodeproj/project.pbxproj index 037563bc..74b55a76 100644 --- a/fish.xcodeproj/project.pbxproj +++ b/fish.xcodeproj/project.pbxproj @@ -1327,7 +1327,7 @@ "SYSCONFDIR=L\\\"/usr/local/etc\\\"", "BINDIR=L\\\"/usr/local/bin\\\"", "DOCDIR=L\\\"/usr/local/share/doc\\\"", - "FISH_BUILD_VERSION=\\\"2.2b1\\\"", + "FISH_BUILD_VERSION=\\\"2.2b1-git\\\"", ); GCC_WARN_64_TO_32_BIT_CONVERSION = YES; GCC_WARN_ABOUT_RETURN_TYPE = YES; @@ -1455,7 +1455,7 @@ "SYSCONFDIR=L\\\"/usr/local/etc\\\"", "BINDIR=L\\\"/usr/local/bin\\\"", "DOCDIR=L\\\"/usr/local/share/doc\\\"", - "FISH_BUILD_VERSION=\\\"2.2b1\\\"", + "FISH_BUILD_VERSION=\\\"2.2b1-git\\\"", ); GCC_SYMBOLS_PRIVATE_EXTERN = NO; GCC_WARN_64_TO_32_BIT_CONVERSION = YES; @@ -1483,7 +1483,7 @@ "SYSCONFDIR=L\\\"/usr/local/etc\\\"", "BINDIR=L\\\"/usr/local/bin\\\"", "DOCDIR=L\\\"/usr/local/share/doc\\\"", - "FISH_BUILD_VERSION=\\\"2.2b1\\\"", + "FISH_BUILD_VERSION=\\\"2.2b1-git\\\"", ); GCC_WARN_64_TO_32_BIT_CONVERSION = YES; GCC_WARN_ABOUT_RETURN_TYPE = YES; diff --git a/osx/config.h b/osx/config.h index a5b0df28..5a8e6b26 100644 --- a/osx/config.h +++ b/osx/config.h @@ -222,7 +222,7 @@ #define PACKAGE_NAME "fish" /* Define to the full name and version of this package. */ -#define PACKAGE_STRING "fish 2.2b1" +#define PACKAGE_STRING "fish 2.2b1-git" /* Define to the one symbol short name of this package. */ #define PACKAGE_TARNAME "fish" @@ -231,7 +231,7 @@ #define PACKAGE_URL "" /* Define to the version of this package. */ -#define PACKAGE_VERSION "2.2b1" +#define PACKAGE_VERSION "2.2b1-git" /* Define to 1 if you have the ANSI C header files. */ #define STDC_HEADERS 1 diff --git a/tests/function.in b/tests/function.in index e533b25b..747bbcec 100644 --- a/tests/function.in +++ b/tests/function.in @@ -30,3 +30,17 @@ set foo 'bad foo' set bar 'bad bar' set baz 'bad baz' frob + +# Test that -a does not mix up the function name with arguments +# See #2068 +function name1 -a arg1 arg2 ; end +function -a arg1 arg2 name2 ; end +function name3 --argument-names arg1 arg2 ; end +function --argument-names arg1 arg2 name4 ; end +for i in (seq 4) + if functions -q name$i + echo "Function name$i found" + else + echo "Function name$i not found, but should have been" + end +end diff --git a/tests/function.out b/tests/function.out index 3fa70990..5a3da619 100644 --- a/tests/function.out +++ b/tests/function.out @@ -18,3 +18,7 @@ $bar: (5) 4: '' 5: '3' $baz: (0) +Function name1 found +Function name2 found +Function name3 found +Function name4 found |