From 208be0f4d4f42abe8e01eae0e41e22e8ac6bae0f Mon Sep 17 00:00:00 2001 From: Christopher Nilsson Date: Wed, 8 Sep 2010 03:31:05 +1000 Subject: [PATCH 1/4] Adding '--rename' option to 'functions' builtin. Aim is to allow an existing function to be renamed, allowing some basic function chaining. Example: > function foo echo Hello end > foo Hello > functions --rename foo bar > foo fish: Unknown command 'foo' > bar Hello > functions --rename fish_prompt old_prompt > function fish_prompt printf "{Boo!}%s" (old_prompt) end {Boo!}> Note in the last case, the new fish_prompt is calling its old definition. --- builtin.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++------ function.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ function.h | 11 +++++++++ 3 files changed, 134 insertions(+), 7 deletions(-) diff --git a/builtin.c b/builtin.c index fe59ce292..7b73d24f3 100644 --- a/builtin.c +++ b/builtin.c @@ -1252,6 +1252,7 @@ static int builtin_functions( wchar_t **argv ) int show_hidden=0; int res = STATUS_BUILTIN_OK; int query = 0; + int rename = 0; woptind=0; @@ -1282,6 +1283,10 @@ static int builtin_functions( wchar_t **argv ) L"query", no_argument, 0, 'q' } , + { + L"rename", no_argument, 0, 'r' + } + , { 0, 0, 0, 0 } @@ -1294,7 +1299,7 @@ static int builtin_functions( wchar_t **argv ) int opt = wgetopt_long( argc, argv, - L"ed:nahq", + L"ed:nahqr", long_options, &opt_index ); if( opt == -1 ) @@ -1305,10 +1310,10 @@ static int builtin_functions( wchar_t **argv ) case 0: if(long_options[opt_index].flag != 0) break; - sb_printf( sb_err, - BUILTIN_ERR_UNKNOWN, - argv[0], - long_options[opt_index].name ); + sb_printf( sb_err, + BUILTIN_ERR_UNKNOWN, + argv[0], + long_options[opt_index].name ); builtin_print_help( argv[0], sb_err ); @@ -1338,6 +1343,10 @@ static int builtin_functions( wchar_t **argv ) query = 1; break; + case 'r': + rename = 1; + break; + case '?': builtin_unknown_option( argv[0], argv[woptind-1] ); return STATUS_BUILTIN_ERROR; @@ -1347,9 +1356,9 @@ static int builtin_functions( wchar_t **argv ) } /* - Erase, desc, query and list are mutually exclusive + Erase, desc, query, rename and list are mutually exclusive */ - if( (erase + (!!desc) + list + query) > 1 ) + if( (erase + (!!desc) + list + query + rename) > 1 ) { sb_printf( sb_err, _( L"%ls: Invalid combination of options\n" ), @@ -1434,6 +1443,50 @@ static int builtin_functions( wchar_t **argv ) al_destroy( &names ); return STATUS_BUILTIN_OK; } + else if( rename ) + { + wchar_t *current_func; + wchar_t *new_func; + + if( argc-woptind != 2 ) + { + sb_printf( sb_err, + _( L"%ls: Expected exactly two names (current function name, and new function name)\n" ), + argv[0] ); + builtin_print_help ( argv[0], sb_err ); + + return STATUS_BUILTIN_ERROR; + } + current_func = argv[woptind]; + new_func = argv[woptind+1]; + + if( !function_exists( current_func ) ) + { + sb_printf( sb_err, + _( L"%ls: Function '%ls' does not exist\n" ), + argv[0], + current_func ); + builtin_print_help( argv[0], sb_err ); + + return STATUS_BUILTIN_ERROR; + } + + // keep things simple: don't allow existing names to be rename targets. + if( function_exists( new_func ) ) + { + sb_printf( sb_err, + _( L"%ls: Function '%ls' already exists. Cannot rename '%ls'\n" ), + argv[0], + new_func, + current_func ); + builtin_print_help( argv[0], sb_err ); + + return STATUS_BUILTIN_ERROR; + } + + function_rename( current_func, new_func ); + return STATUS_BUILTIN_OK; + } for( i=woptind; idefinition_offset = orig_d->definition_offset; + d->definition = halloc_wcsdup( d, orig_d->definition ); + if( orig_d->named_arguments ) + { + d->named_arguments = al_halloc( d ); + for( i=0; inamed_arguments ); i++ ) + { + al_push( d->named_arguments, halloc_wcsdup( d, (wchar_t *)al_get( orig_d->named_arguments, i ) ) ); + } + d->description = orig_d->description?halloc_wcsdup(d, orig_d->description):0; + d->definition_file = 0; + d->is_autoload = 0; + d->shadows = orig_d->shadows; + } + + hash_put( &function, intern(new_name), d ); + + // wire up the same events... if any. + ev.type = EVENT_ANY; + ev.function_name = name; + event_get(&ev, fn_events); + + if( fn_events ) + { + ev.function_name = new_name; + + for( i=0; itype; + ev.param1 = orig_ev->param1; + event_add_handler( &ev ); + } + } + return 1; +} + +void function_rename( const wchar_t *name, const wchar_t *new_name ) +{ + if( function_copy( name, new_name ) ) + function_remove( name ); +} + + int function_exists( const wchar_t *cmd ) { diff --git a/function.h b/function.h index 7e1729486..31243663f 100644 --- a/function.h +++ b/function.h @@ -131,4 +131,15 @@ array_list_t *function_get_named_arguments( const wchar_t *name ); */ int function_get_shadows( const wchar_t *name ); +/** + Creates a new function using the same definition as the specified function. + Returns non-zero if copy is successful. +*/ +int function_copy( const wchar_t *name, const wchar_t *new_name ); + +/** + Renames the specified function. +*/ +void function_rename( const wchar_t *name, const wchar_t *new_name ); + #endif From 7914c92824dfbb07466a37f12c7a06c7dd5f4de4 Mon Sep 17 00:00:00 2001 From: Christopher Nilsson Date: Thu, 9 Sep 2010 23:48:18 +1000 Subject: [PATCH 2/4] replaced the functions '--rename' option with '--copy'. Copying the function implementation was the main point. Actually removing the original isn't necessary, as that functionality already exists (functions -e). --- builtin.c | 25 +++++++++++++------------ function.c | 32 ++++---------------------------- function.h | 5 ----- 3 files changed, 17 insertions(+), 45 deletions(-) diff --git a/builtin.c b/builtin.c index 7b73d24f3..ff849f054 100644 --- a/builtin.c +++ b/builtin.c @@ -1252,7 +1252,7 @@ static int builtin_functions( wchar_t **argv ) int show_hidden=0; int res = STATUS_BUILTIN_OK; int query = 0; - int rename = 0; + int copy = 0; woptind=0; @@ -1284,7 +1284,7 @@ static int builtin_functions( wchar_t **argv ) } , { - L"rename", no_argument, 0, 'r' + L"copy", no_argument, 0, 'c' } , { @@ -1299,7 +1299,7 @@ static int builtin_functions( wchar_t **argv ) int opt = wgetopt_long( argc, argv, - L"ed:nahqr", + L"ed:nahqc", long_options, &opt_index ); if( opt == -1 ) @@ -1343,8 +1343,8 @@ static int builtin_functions( wchar_t **argv ) query = 1; break; - case 'r': - rename = 1; + case 'c': + copy = 1; break; case '?': @@ -1356,9 +1356,9 @@ static int builtin_functions( wchar_t **argv ) } /* - Erase, desc, query, rename and list are mutually exclusive + Erase, desc, query, copy and list are mutually exclusive */ - if( (erase + (!!desc) + list + query + rename) > 1 ) + if( (erase + (!!desc) + list + query + copy) > 1 ) { sb_printf( sb_err, _( L"%ls: Invalid combination of options\n" ), @@ -1443,7 +1443,7 @@ static int builtin_functions( wchar_t **argv ) al_destroy( &names ); return STATUS_BUILTIN_OK; } - else if( rename ) + else if( copy ) { wchar_t *current_func; wchar_t *new_func; @@ -1471,11 +1471,11 @@ static int builtin_functions( wchar_t **argv ) return STATUS_BUILTIN_ERROR; } - // keep things simple: don't allow existing names to be rename targets. + // keep things simple: don't allow existing names to be copy targets. if( function_exists( new_func ) ) { sb_printf( sb_err, - _( L"%ls: Function '%ls' already exists. Cannot rename '%ls'\n" ), + _( L"%ls: Function '%ls' already exists. Cannot create copy '%ls'\n" ), argv[0], new_func, current_func ); @@ -1484,8 +1484,9 @@ static int builtin_functions( wchar_t **argv ) return STATUS_BUILTIN_ERROR; } - function_rename( current_func, new_func ); - return STATUS_BUILTIN_OK; + if( function_copy( current_func, new_func ) ) + return STATUS_BUILTIN_OK; + return STATUS_BUILTIN_ERROR; } for( i=woptind; inamed_arguments, halloc_wcsdup( d, (wchar_t *)al_get( orig_d->named_arguments, i ) ) ); } d->description = orig_d->description?halloc_wcsdup(d, orig_d->description):0; + d->shadows = orig_d->shadows; + + // This new instance of the function shouldn't be tied to the def + // file of the original. d->definition_file = 0; d->is_autoload = 0; - d->shadows = orig_d->shadows; } hash_put( &function, intern(new_name), d ); - // wire up the same events... if any. - ev.type = EVENT_ANY; - ev.function_name = name; - event_get(&ev, fn_events); - - if( fn_events ) - { - ev.function_name = new_name; - - for( i=0; itype; - ev.param1 = orig_ev->param1; - event_add_handler( &ev ); - } - } return 1; } -void function_rename( const wchar_t *name, const wchar_t *new_name ) -{ - if( function_copy( name, new_name ) ) - function_remove( name ); -} - int function_exists( const wchar_t *cmd ) { diff --git a/function.h b/function.h index 31243663f..b871fa0c6 100644 --- a/function.h +++ b/function.h @@ -137,9 +137,4 @@ int function_get_shadows( const wchar_t *name ); */ int function_copy( const wchar_t *name, const wchar_t *new_name ); -/** - Renames the specified function. -*/ -void function_rename( const wchar_t *name, const wchar_t *new_name ); - #endif From 5c9b42e260fb8797cad138d3ead0810fc72f01b2 Mon Sep 17 00:00:00 2001 From: Christopher Nilsson Date: Sun, 12 Sep 2010 13:16:11 +1000 Subject: [PATCH 3/4] 'functions --copy': added sanity check on new function name. Now matches function create behaviour, running the new function name through wcsfuncname() and parser_keywords_is_reserved(), before allowing the copy. --- builtin.c | 10 ++++++++++ function.c | 2 -- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/builtin.c b/builtin.c index ff849f054..0d1139620 100644 --- a/builtin.c +++ b/builtin.c @@ -1471,6 +1471,16 @@ static int builtin_functions( wchar_t **argv ) return STATUS_BUILTIN_ERROR; } + if( (wcsfuncname( new_func ) != 0) || parser_keywords_is_reserved( new_func ) ) + { + sb_printf( sb_err, + _( L"%ls: Illegal function name '%ls'\n"), + argv[0], + new_func ); + builtin_print_help( argv[0], sb_err ); + return STATUS_BUILTIN_ERROR; + } + // keep things simple: don't allow existing names to be copy targets. if( function_exists( new_func ) ) { diff --git a/function.c b/function.c index 5b2dcd5a7..2ca736dce 100644 --- a/function.c +++ b/function.c @@ -226,8 +226,6 @@ int function_copy( const wchar_t *name, const wchar_t *new_name ) CHECK( name, 0 ); CHECK( new_name, 0 ); - fn_events = 0; - orig_d = (function_internal_data_t *)hash_get(&function, name); if( !orig_d ) return 0; From ec8b3593f36908e19d15197b550a171938358b97 Mon Sep 17 00:00:00 2001 From: Christopher Nilsson Date: Sun, 12 Sep 2010 20:29:34 +1000 Subject: [PATCH 4/4] added '-c' option to the functions.txt docs. --- doc_src/functions.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc_src/functions.txt b/doc_src/functions.txt index 7643bfc68..b9057e18b 100644 --- a/doc_src/functions.txt +++ b/doc_src/functions.txt @@ -8,6 +8,7 @@ This builtin command is used to print or erase functions. - -a or --all list all functions, even those whose name start with an underscore. +- -c OLDNAME NEWNAME or --copy OLDNAME NEWNAME creates a new function named NEWNAME, using the definition of the OLDNAME function. - -d DESCRIPTION or --description=DESCRIPTION change the description of this function - -e or --erase causes the specified functions to be erased. - -h or --help display a help message and exit @@ -23,5 +24,8 @@ Automatically loaded functions can not be removed using functions -e. Either remove the definition file or change the $fish_function_path variable to remove autoloaded functions. +Function copies, created with -c, will not have any event/signal/on-exit +notifications that the original may have had. + The exit status of the functions builtin is the number functions specified in the argument list that do not exist.