diff --git a/doc_src/function.txt b/doc_src/function.txt index 65cbdca98..e58d6513a 100644 --- a/doc_src/function.txt +++ b/doc_src/function.txt @@ -29,8 +29,6 @@ The following options are available: - `-s` or `--on-signal SIGSPEC` tells fish to run this function when the signal SIGSPEC is delivered. SIGSPEC can be a signal number, or the signal name, such as SIGHUP (or just HUP). -- `-B` or `--shadow-builtin` must be specified if the function name is the same as a builtin. Specifying this flag indicates your acknowledgement that you are wrapping or replacing the builtin command. This is a safety feature to make it harder for people to inadvertently break the shell by doing things like `function test; return 0; end`. If the function name is not currently a builtin using this flag will produce an error. If you want to write a function that provides a builtin to an older version of fish you need to add something like `builtin --names | grep -q '^cmd$'; and return` to the top of the function script (where `cmd` is the name of the builtin/function). That will keep your script from replacing the builtin with your function on the newer fish version while allowing your function to provide similar functionality on older versions of fish. - - `-S` or `--no-scope-shadowing` allows the function to access the variables of calling functions. Normally, any variables inside the function that have the same name as variables from the calling function are "shadowed", and their contents is independent of the calling function. - `-V` or `--inherit-variable NAME` snapshots the value of the variable `NAME` and defines a local variable with that same name and value when the function is executed. diff --git a/share/functions/__fish_config_interactive.fish b/share/functions/__fish_config_interactive.fish index fd206d45e..b9b8638e7 100644 --- a/share/functions/__fish_config_interactive.fish +++ b/share/functions/__fish_config_interactive.fish @@ -310,7 +310,7 @@ function __fish_config_interactive -d "Initializations that should be performed # Don't allow setting color other than what linux offers (see #2001) functions -e set_color - function set_color --shadow-builtin + function set_color set -l term_colors black red green yellow blue magenta cyan white normal for a in $argv if not contains -- $a $term_colors diff --git a/share/functions/cd.fish b/share/functions/cd.fish index fdc8b607f..adf0327ca 100644 --- a/share/functions/cd.fish +++ b/share/functions/cd.fish @@ -1,7 +1,7 @@ # # Wrap the builtin cd command to maintain directory history. # -function cd --shadow-builtin --description "Change directory" +function cd --description "Change directory" set -l MAX_DIR_HIST 25 if test (count $argv) -gt 1 diff --git a/share/functions/history.fish b/share/functions/history.fish index 8b5a94fd3..d855de823 100644 --- a/share/functions/history.fish +++ b/share/functions/history.fish @@ -1,7 +1,7 @@ # # Wrap the builtin history command to provide additional functionality. # -function history --shadow-builtin --description "display or manipulate interactive command history" +function history --description "display or manipulate interactive command history" set -l cmd set -l search_mode set -l with_time @@ -40,7 +40,7 @@ function history --shadow-builtin --description "display or manipulate interacti end if not set -q cmd[1] - set cmd search # default to "search" if the user didn't explicitly specify a command + set cmd search # default to "search" if the user didn't explicitly specify a command else if set -q cmd[2] printf (_ "You cannot specify multiple commands: %s\n") "$cmd" return 1 @@ -60,7 +60,7 @@ function history --shadow-builtin --description "display or manipulate interacti builtin history --search $search_mode $with_time -- $argv end - case delete # Interactively delete history + case delete # Interactively delete history # TODO: Fix this to deal with history entries that have multiple lines. if not set -q argv[1] printf (_ "You must specify at least one search term when deleting entries") >&2 @@ -105,8 +105,8 @@ function history --shadow-builtin --description "display or manipulate interacti for i in (string split " " -- $choice) if test -z "$i" - or not string match -qr '^[1-9][0-9]*$' -- $i - or test $i -gt $found_items_count + or not string match -qr '^[1-9][0-9]*$' -- $i + or test $i -gt $found_items_count printf "Ignoring invalid history entry ID \"%s\"\n" $i continue end @@ -130,7 +130,7 @@ function history --shadow-builtin --description "display or manipulate interacti # Erase the entire history. read --local --prompt "echo 'Are you sure you want to clear history? (y/n) '" choice if test "$choice" = "y" - or test "$choice" = "yes" + or test "$choice" = "yes" builtin history --clear -- $argv and echo "History cleared!" end diff --git a/src/builtin.cpp b/src/builtin.cpp index b317614cb..53c856c2d 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -920,10 +920,6 @@ static wcstring functions_def(const wcstring &name) { out.append(esc_desc); } - if (function_get_shadow_builtin(name)) { - out.append(L" --shadow-builtin"); - } - if (!function_get_shadow_scope(name)) { out.append(L" --no-scope-shadowing"); } @@ -1492,17 +1488,13 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis // Hackish const_cast matches the one in builtin_run. const null_terminated_array_t argv_array(args); wchar_t **argv = const_cast(argv_array.get()); - int argc = builtin_count_args(argv); int res = STATUS_BUILTIN_OK; wchar_t *desc = 0; std::vector events; - bool has_named_arguments = false; wcstring_list_t named_arguments; wcstring_list_t inherit_vars; - - bool shadow_builtin = false; bool shadow_scope = true; wcstring_list_t wrap_targets; @@ -1523,7 +1515,6 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis {L"wraps", required_argument, 0, 'w'}, {L"help", no_argument, 0, 'h'}, {L"argument-names", no_argument, 0, 'a'}, - {L"shadow-builtin", no_argument, 0, 'B'}, {L"no-scope-shadowing", no_argument, 0, 'S'}, {L"inherit-variable", required_argument, 0, 'V'}, {0, 0, 0, 0}}; @@ -1532,7 +1523,7 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis int opt_index = 0; // The leading - here specifies RETURN_IN_ORDER. - int opt = w.wgetopt_long(argc, argv, L"-d:s:j:p:v:e:w:haBSV:", long_options, &opt_index); + int opt = w.wgetopt_long(argc, argv, L"-d:s:j:p:v:e:w:haSV:", long_options, &opt_index); if (opt == -1) break; switch (opt) { case 0: { @@ -1628,10 +1619,6 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis name_is_first_positional = !positionals.empty(); break; } - case 'B': { - shadow_builtin = true; - break; - } case 'S': { shadow_scope = false; break; @@ -1720,30 +1707,6 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis } } - if (!res) { - bool function_name_shadows_builtin = false; - wcstring_list_t builtin_names = builtin_get_names(); - for (size_t i = 0; i < builtin_names.size(); i++) { - const wchar_t *el = builtin_names.at(i).c_str(); - if (el == function_name) { - function_name_shadows_builtin = true; - break; - } - } - if (function_name_shadows_builtin && !shadow_builtin) { - append_format( - *out_err, - _(L"%ls: function name shadows a builtin so you must use '--shadow-builtin'"), - argv[0]); - res = STATUS_BUILTIN_ERROR; - } else if (!function_name_shadows_builtin && shadow_builtin) { - append_format(*out_err, _(L"%ls: function name does not shadow a builtin so you " - L"must not use '--shadow-builtin'"), - argv[0]); - res = STATUS_BUILTIN_ERROR; - } - } - if (!res) { // Here we actually define the function! function_data_t d; @@ -1751,7 +1714,6 @@ int builtin_function(parser_t &parser, io_streams_t &streams, const wcstring_lis d.name = function_name; if (desc) d.description = desc; d.events.swap(events); - d.shadow_builtin = shadow_builtin; d.shadow_scope = shadow_scope; d.named_arguments.swap(named_arguments); d.inherit_vars.swap(inherit_vars); diff --git a/src/exec.cpp b/src/exec.cpp index f367396fb..7d826766d 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -633,7 +633,6 @@ void exec_job(parser_t &parser, job_t *j) { const wcstring func_name = p->argv0(); wcstring def; bool function_exists = function_get_definition(func_name, &def); - bool shadow_scope = function_get_shadow_scope(func_name); const std::map inherit_vars = function_get_inherit_vars(func_name); diff --git a/src/function.cpp b/src/function.cpp index 24db8cc9c..6b8d4c7fb 100644 --- a/src/function.cpp +++ b/src/function.cpp @@ -144,7 +144,6 @@ function_info_t::function_info_t(const function_data_t &data, const wchar_t *fil named_arguments(data.named_arguments), inherit_vars(snapshot_vars(data.inherit_vars)), is_autoload(autoload), - shadow_builtin(data.shadow_builtin), shadow_scope(data.shadow_scope) {} function_info_t::function_info_t(const function_info_t &data, const wchar_t *filename, @@ -156,7 +155,6 @@ function_info_t::function_info_t(const function_info_t &data, const wchar_t *fil named_arguments(data.named_arguments), inherit_vars(data.inherit_vars), is_autoload(autoload), - shadow_builtin(data.shadow_builtin), shadow_scope(data.shadow_scope) {} void function_add(const function_data_t &data, const parser_t &parser, int definition_line_offset) { @@ -260,12 +258,6 @@ std::map function_get_inherit_vars(const wcstring &name) { return func ? func->inherit_vars : std::map(); } -int function_get_shadow_builtin(const wcstring &name) { - scoped_lock locker(functions_lock); - const function_info_t *func = function_get(name); - return func ? func->shadow_builtin : false; -} - int function_get_shadow_scope(const wcstring &name) { scoped_lock locker(functions_lock); const function_info_t *func = function_get(name); diff --git a/src/function.h b/src/function.h index bd193104c..c104f080d 100644 --- a/src/function.h +++ b/src/function.h @@ -32,8 +32,6 @@ struct function_data_t { wcstring_list_t inherit_vars; /// Set to true if invoking this function shadows the variables of the underlying function. bool shadow_scope; - /// Set to true if this function shadows a builtin. - bool shadow_builtin; }; class function_info_t { @@ -53,8 +51,6 @@ class function_info_t { const std::map inherit_vars; /// Flag for specifying that this function was automatically loaded. const bool is_autoload; - /// Set to true if this function shadows a builtin. - const bool shadow_builtin; /// Set to true if invoking this function shadows the variables of the underlying function. const bool shadow_scope; @@ -129,9 +125,6 @@ std::map function_get_inherit_vars(const wcstring &name); /// is successful. bool function_copy(const wcstring &name, const wcstring &new_name); -/// Returns whether this function shadows a builtin of the same name. -int function_get_shadow_builtin(const wcstring &name); - /// Returns whether this function shadows variables of the underlying function. int function_get_shadow_scope(const wcstring &name); diff --git a/tests/function.err b/tests/function.err index 72671ac96..e69de29bb 100644 --- a/tests/function.err +++ b/tests/function.err @@ -1,8 +0,0 @@ -function: function name shadows a builtin so you must use '--shadow-builtin' -fish: function pwd; end - ^ -yes, it failed as expected -function: function name does not shadow a builtin so you must not use '--shadow-builtin' -fish: function not_builtin --shadow-builtin; end - ^ -yes, it failed as expected diff --git a/tests/function.in b/tests/function.in index 8202fb139..747bbcecb 100644 --- a/tests/function.in +++ b/tests/function.in @@ -44,16 +44,3 @@ for i in (seq 4) echo "Function name$i not found, but should have been" end end - -# Test that we can't define a function that shadows a builtin by accident. -function pwd; end -or echo 'yes, it failed as expected' >&2 - -# Test that we can define a function that shadows a builtin if we use the -# right flag. -function pwd --shadow-builtin; end -and echo '"function pwd --shadow-builtin" worked' - -# Using --shadow-builtin for a non-builtin function name also fails. -function not_builtin --shadow-builtin; end -or echo 'yes, it failed as expected' >&2 diff --git a/tests/function.out b/tests/function.out index 0d12479ed..5a3da6195 100644 --- a/tests/function.out +++ b/tests/function.out @@ -22,4 +22,3 @@ Function name1 found Function name2 found Function name3 found Function name4 found -"function pwd --shadow-builtin" worked