* Defining function based on alias @ 2017-01-07 22:16 Peter Stephenson 2017-01-08 19:19 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Peter Stephenson @ 2017-01-07 22:16 UTC (permalink / raw) To: Zsh hackers list It's pretty much always an error to define a function when the name of the function is being expanded as an alias, right? So we could make it an error, possible with an option though this would need to be active by default to make it useful. % foo() { echo this is function foo; } % alias foo='echo bar' % foo() { echo this is not function foo; } zsh: defining function based on alias `foo' zsh: parse error near `()' pws diff --git a/Src/input.c b/Src/input.c index eb968ea..f2c22c8 100644 --- a/Src/input.c +++ b/Src/input.c @@ -670,3 +670,22 @@ ingetptr(void) { return inbufptr; } + +/**/ +char *input_hasalias(void) +{ + int flags = inbufflags; + struct instacks *instackptr = instacktop; + + for (;;) + { + if (!(flags & INP_CONT)) + break; + instackptr--; + if (instackptr->alias) + return instackptr->alias->node.nam; + flags = instackptr->flags; + } + + return NULL; +} diff --git a/Src/parse.c b/Src/parse.c index 50a0d5f..1fed020 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -1738,6 +1738,7 @@ par_simple(int *cmplx, int nr) { int oecused = ecused, isnull = 1, r, argc = 0, p, isfunc = 0, sr = 0; int c = *cmplx, nrediradd, assignments = 0, ppost = 0, is_typeset = 0; + char *hasalias = input_hasalias(); wordcode postassigns = 0; r = ecused; @@ -1809,6 +1810,8 @@ par_simple(int *cmplx, int nr) } else break; zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } if (tok == AMPER || tok == AMPERBANG) YYERROR(oecused); @@ -1839,6 +1842,8 @@ par_simple(int *cmplx, int nr) char *idstring = dupstrpfx(tokstr+1, eptr-tokstr-1); redir_var = 1; zshlex(); + if (!hasalias) + hasalias = input_hasalias(); if (IS_REDIROP(tok) && tokfd == -1) { @@ -1874,6 +1879,8 @@ par_simple(int *cmplx, int nr) argc++; } zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } } else if (IS_REDIROP(tok)) { *cmplx = c = 1; @@ -1902,6 +1909,8 @@ par_simple(int *cmplx, int nr) ecstr(name); ecstr(str); zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } else if (tok == ENVARRAY) { int n, parr; @@ -1936,6 +1945,10 @@ par_simple(int *cmplx, int nr) /* Error if preceding assignments */ if (assignments || postassigns) YYERROR(oecused); + if (hasalias && argc) { + zwarn("defining function based on alias `%s'", hasalias); + YYERROR(oecused); + } *cmplx = c; lineno = 0; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-07 22:16 Defining function based on alias Peter Stephenson @ 2017-01-08 19:19 ` Peter Stephenson 2017-01-08 20:14 ` Bart Schaefer ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Peter Stephenson @ 2017-01-08 19:19 UTC (permalink / raw) To: Zsh hackers list On Sat, 7 Jan 2017 22:16:59 +0000 Peter Stephenson <p.w.stephenson@ntlworld.com> wrote: > It's pretty much always an error to define a function when the name of > the function is being expanded as an alias, right? So we could make it > an error, possible with an option though this would need to be active by > default to make it useful. > > % foo() { echo this is function foo; } > % alias foo='echo bar' > % foo() { echo this is not function foo; } > zsh: defining function based on alias `foo' > zsh: parse error near `()' OK, let me rephrase this. I am about to commit the following, which I hope will hope will stop people coming to grief with this common confusion. Please say if you see any problems. I have changed the code so that it doesn't complain if the alias in affect at the start of the definition provided the "()" as well, since there is no confusion in this case. I have added an option to turn this off which is active in sh compatibility mode since I would guess we can't get away with this behaviour there. diff --git a/Doc/Zsh/options.yo b/Doc/Zsh/options.yo index f68a945..ba3ec17 100644 --- a/Doc/Zsh/options.yo +++ b/Doc/Zsh/options.yo @@ -1539,6 +1539,35 @@ enditem() subsect(Scripts and Functions) startitem() +pindex(ALIAS_FUNC_DEF) +pindex(NO_ALIAS_FUNC_DEF) +pindex(ALIASFUNCDEF) +pindex(NOALIASFUNCDEF) +cindex(functions, defining with expanded aliases) +cindex(aliases, expanding in function definition) +item(tt(ALIAS_FUNC_DEF) <S>)( +By default, zsh does not allow the definition of functions using +the `var(name) tt(LPAR()RPAR())' syntax if var(name) was expanded as an +alias: this causes an error. This is usually the desired behaviour, as +otherwise the combination of an alias and a function based on the same +definition can easily cause problems. + +When this option is set, aliases can be used for defining functions. + +For example, consider the following definitions. + +example(alias foo=bar +foo+LPAR()RPAR() { + print This probably does not do what you expect. +}) + +Here, tt(foo) is expanded as an alias to tt(bar) before the +tt(LPAR()RPAR()) is encountered, so the function defined would be named +tt(bar). By default this is instead an error in native mode. Note that +quoting any part of the function name, or using the keyword +tt(function), avoids the problem, so is recommended when the function +name can also be an alias. +) pindex(C_BASES) pindex(NO_C_BASES) pindex(CBASES) diff --git a/Src/input.c b/Src/input.c index eb968ea..f2c22c8 100644 --- a/Src/input.c +++ b/Src/input.c @@ -670,3 +670,22 @@ ingetptr(void) { return inbufptr; } + +/**/ +char *input_hasalias(void) +{ + int flags = inbufflags; + struct instacks *instackptr = instacktop; + + for (;;) + { + if (!(flags & INP_CONT)) + break; + instackptr--; + if (instackptr->alias) + return instackptr->alias->node.nam; + flags = instackptr->flags; + } + + return NULL; +} diff --git a/Src/options.c b/Src/options.c index 18619c8..4729ba5 100644 --- a/Src/options.c +++ b/Src/options.c @@ -78,6 +78,7 @@ mod_export HashTable optiontab; */ static struct optname optns[] = { {{NULL, "aliases", OPT_EMULATE|OPT_ALL}, ALIASESOPT}, +{{NULL, "aliasfuncdef", OPT_EMULATE|OPT_BOURNE}, ALIASFUNCDEF}, {{NULL, "allexport", OPT_EMULATE}, ALLEXPORT}, {{NULL, "alwayslastprompt", OPT_ALL}, ALWAYSLASTPROMPT}, {{NULL, "alwaystoend", 0}, ALWAYSTOEND}, diff --git a/Src/parse.c b/Src/parse.c index 50a0d5f..ed6c4a8 100644 --- a/Src/parse.c +++ b/Src/parse.c @@ -1738,6 +1738,7 @@ par_simple(int *cmplx, int nr) { int oecused = ecused, isnull = 1, r, argc = 0, p, isfunc = 0, sr = 0; int c = *cmplx, nrediradd, assignments = 0, ppost = 0, is_typeset = 0; + char *hasalias = input_hasalias(); wordcode postassigns = 0; r = ecused; @@ -1809,6 +1810,8 @@ par_simple(int *cmplx, int nr) } else break; zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } if (tok == AMPER || tok == AMPERBANG) YYERROR(oecused); @@ -1839,6 +1842,8 @@ par_simple(int *cmplx, int nr) char *idstring = dupstrpfx(tokstr+1, eptr-tokstr-1); redir_var = 1; zshlex(); + if (!hasalias) + hasalias = input_hasalias(); if (IS_REDIROP(tok) && tokfd == -1) { @@ -1874,6 +1879,8 @@ par_simple(int *cmplx, int nr) argc++; } zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } } else if (IS_REDIROP(tok)) { *cmplx = c = 1; @@ -1902,6 +1909,8 @@ par_simple(int *cmplx, int nr) ecstr(name); ecstr(str); zshlex(); + if (!hasalias) + hasalias = input_hasalias(); } else if (tok == ENVARRAY) { int n, parr; @@ -1936,6 +1945,11 @@ par_simple(int *cmplx, int nr) /* Error if preceding assignments */ if (assignments || postassigns) YYERROR(oecused); + if (hasalias && !isset(ALIASFUNCDEF) && argc && + hasalias != input_hasalias()) { + zwarn("defining function based on alias `%s'", hasalias); + YYERROR(oecused); + } *cmplx = c; lineno = 0; diff --git a/Src/zsh.h b/Src/zsh.h index f22d8b1..2a41638 100644 --- a/Src/zsh.h +++ b/Src/zsh.h @@ -2222,6 +2222,7 @@ struct histent { enum { OPT_INVALID, ALIASESOPT, + ALIASFUNCDEF, ALLEXPORT, ALWAYSLASTPROMPT, ALWAYSTOEND, diff --git a/Test/A02alias.ztst b/Test/A02alias.ztst index e52578e..e68e93e 100644 --- a/Test/A02alias.ztst +++ b/Test/A02alias.ztst @@ -82,6 +82,7 @@ 0:Global aliasing quotes > a string S *>*5*echo S a string S " +# " # Note there is a trailing space on the "> a string S " line ( @@ -115,3 +116,24 @@ 1:error message has the correct sign ?(eval):alias:1: bad option: +x ?(eval):alias:1: bad option: -z + + # Usual issue that aliases aren't expanded until we + # trigger a new parse... + (alias badalias=notacommand + eval 'badalias() { print does not work; }') +1:ALIAS_FUNC_DEF off by default. +?(eval):1: defining function based on alias `badalias' +?(eval):1: parse error near `()' + + (alias goodalias=isafunc + setopt ALIAS_FUNC_DEF + eval 'goodalias() { print does now work; }' + isafunc) +0:ALIAS_FUNC_DEF causes the icky behaviour to be avaliable +>does now work + + (alias thisisokthough='thisworks() { print That worked; }' + eval thisisokthough + thisworks) +0:NO_ALIAS_FUNC_DEF works if the alias is a complete definition +>That worked ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-08 19:19 ` Peter Stephenson @ 2017-01-08 20:14 ` Bart Schaefer 2017-01-08 20:23 ` Peter Stephenson 2017-01-09 0:09 ` Bart Schaefer 2017-01-09 1:55 ` Daniel Shahaf 2 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2017-01-08 20:14 UTC (permalink / raw) To: Zsh hackers list On Jan 8, 7:19pm, Peter Stephenson wrote: } Subject: Re: Defining function based on alias } } I am about to commit the following, which I hope will hope will stop } people coming to grief with this common confusion. Please say if you } see any problems. No problems, but -- if it's possible to detect this situation, why not simply suppress the alias expansion and define the function as named, rather than trigger an error? Etc/FAQ should be updated too. I never remember how to rebuild FAQ from FAQ.yo ... can someone check my yodl syntax? diff --git a/Etc/FAQ.yo b/Etc/FAQ.yo index 6b635ea..9eb23db 100644 --- a/Etc/FAQ.yo +++ b/Etc/FAQ.yo @@ -574,7 +574,8 @@ tt(EXTENDED_GLOB). it() Aliases and functions: itemize( it() The order in which aliases and functions are defined is significant: - function definitions with () expand aliases -- see question \ + in zsh versions 5.3.1 and earlier, function definitions with () + expand aliases -- see question \ link(2.3)(23). it() Aliases and functions cannot be exported. it() There are no tracked aliases: command hashing replaces these. @@ -704,6 +705,12 @@ label(23) the manual). Note also that the mytt(;) at the end of the function is optional in zsh, but not in ksh or sh (for sh's where it exists). + Recent versions of zsh support anonymous functions with argument lists, + so you can create an alias that acts like a function. + verb( + alias cd='() { builtin cd "$@"; print -D $PWD; }' + ) + Here is Bart Schaefer's guide to converting csh aliases for zsh. enumerate( @@ -711,6 +718,9 @@ label(23) then in zsh you need a function (referencing tt($1), tt($*) etc.). Otherwise, you can use a zsh alias. + In zsh version 5.0 and later you can use an anonymous zsh function in + combination with a zsh alias. + myeit() If you use a zsh function, you need to refer _at_least_ to tt($*) in the body (inside the tt({ })). Parameters don't magically appear inside the tt({ }) the way they get appended to an alias. @@ -772,13 +782,17 @@ label(23) alias l='/bin/ls -F' l() { /bin/ls -la "$@" | more } ) - mytt(l) in the function definition is in command position and is expanded + mytt(l) in the function definition is in command position and may expand as an alias, defining mytt(/bin/ls) and mytt(-F) as functions which call mytt(/bin/ls), which gets a bit recursive. This can be avoided if you use - mytt(function) to define a function, which doesn't expand aliases. It is - possible to argue for extra warnings somewhere in this mess. + mytt(function) to define a function, which doesn't expand aliases. + + For zsh versions after 5.3.1, alias expansion of function names is an + error by default and the option tt(ALIAS_FUNC_DEF) must be active to + expand aliases in function names. However, this option em(is) active + in compatibility modes where aliases are supported. - One workaround for this is to use the "function" keyword instead: + Here is an example using the "function" keyword instead: verb( alias l='/bin/ls -F' function l { /bin/ls -la "$@" | more } ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-08 20:14 ` Bart Schaefer @ 2017-01-08 20:23 ` Peter Stephenson 0 siblings, 0 replies; 8+ messages in thread From: Peter Stephenson @ 2017-01-08 20:23 UTC (permalink / raw) To: Zsh hackers list On Sun, 8 Jan 2017 12:14:54 -0800 Bart Schaefer <schaefer@brasslantern.com> wrote: > On Jan 8, 7:19pm, Peter Stephenson wrote: > } Subject: Re: Defining function based on alias > } > } I am about to commit the following, which I hope will hope will stop > } people coming to grief with this common confusion. Please say if you > } see any problems. > > No problems, but -- if it's possible to detect this situation, why not > simply suppress the alias expansion and define the function as named, > rather than trigger an error? It's too late; we'd have to wind back to where the alias was read and read eveything in again, which is quite contorted, and not actually guaranteed to fix things if it's a tortuous alias. Possibly an even better argument is that this silently changes the syntax, which is usually a bad idea. > I never remember how to rebuild FAQ from FAQ.yo ... can someone check > my yodl syntax? It seems OK but you just run "make" in Etc. pws ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-08 19:19 ` Peter Stephenson 2017-01-08 20:14 ` Bart Schaefer @ 2017-01-09 0:09 ` Bart Schaefer 2017-01-09 4:02 ` Daniel Shahaf 2017-01-09 1:55 ` Daniel Shahaf 2 siblings, 1 reply; 8+ messages in thread From: Bart Schaefer @ 2017-01-09 0:09 UTC (permalink / raw) To: Zsh hackers list On Jan 8, 7:19pm, Peter Stephenson wrote: } } I am about to commit the following, which I hope will hope will stop } people coming to grief with this common confusion. Please say if you } see any problems. OK, here's an unusual case ... alias safely:foo='[[ -n $functions[foo] ]] || foo' safely:foo() { echo this is foo only if no other foo } I hesitate to suggest further convolutions, but maybe an option to the alias command to "locally" switch on ALIAS_FUNC_DEF if it's actually *meant* to be used that way? Probably not very important. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-09 0:09 ` Bart Schaefer @ 2017-01-09 4:02 ` Daniel Shahaf 2017-01-09 10:33 ` Peter Stephenson 0 siblings, 1 reply; 8+ messages in thread From: Daniel Shahaf @ 2017-01-09 4:02 UTC (permalink / raw) To: zsh-workers Bart Schaefer wrote on Sun, Jan 08, 2017 at 16:09:21 -0800: > On Jan 8, 7:19pm, Peter Stephenson wrote: > } > } I am about to commit the following, which I hope will hope will stop > } people coming to grief with this common confusion. Please say if you > } see any problems. > > OK, here's an unusual case ... > > alias safely:foo='[[ -n $functions[foo] ]] || foo' > > safely:foo() { echo this is foo only if no other foo } > > I hesitate to suggest further convolutions, but maybe an option to the > alias command to "locally" switch on ALIAS_FUNC_DEF if it's actually > *meant* to be used that way? This case doesn't require a new option: alias safely:foo='[[ -n $functions[foo] ]] || function foo' > Probably not very important. > Another case: % alias foo='' % foo() echo hey hey Or less minimally: % setopt NO_multifuncdef % alias foo='' % foo bar () : % which -a foo bar foo: aliased to bar () { : } Should these two behave differently when the new option is set? The first case appeared to be a non-anonymous function definition and the second a multifuncdef, but the alias changed the semantics. As Bart said: probably not very important. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-09 4:02 ` Daniel Shahaf @ 2017-01-09 10:33 ` Peter Stephenson 0 siblings, 0 replies; 8+ messages in thread From: Peter Stephenson @ 2017-01-09 10:33 UTC (permalink / raw) To: zsh-workers On Mon, 9 Jan 2017 04:02:58 +0000 Daniel Shahaf <d.s@daniel.shahaf.name> wrote: > Should these two behave differently when the new option is set? The > first case appeared to be a non-anonymous function definition and the > second a multifuncdef, but the alias changed the semantics. I wasn't proposing to try to protect everyone from every possible error, just the case we know is quite common even in normal use of functions and aliases. Conversely, if you need to do clever tricks I'm assuming you will simply sign on the dotted line to say you will take responsibility and set the option. I do need to add something to the README. pws ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Defining function based on alias 2017-01-08 19:19 ` Peter Stephenson 2017-01-08 20:14 ` Bart Schaefer 2017-01-09 0:09 ` Bart Schaefer @ 2017-01-09 1:55 ` Daniel Shahaf 2 siblings, 0 replies; 8+ messages in thread From: Daniel Shahaf @ 2017-01-09 1:55 UTC (permalink / raw) To: Peter Stephenson; +Cc: Zsh hackers list Peter Stephenson wrote on Sun, Jan 08, 2017 at 19:19:22 +0000: > +++ b/Src/input.c > @@ -670,3 +670,22 @@ ingetptr(void) > { > return inbufptr; > } > + > +/**/ > +char *input_hasalias(void) May we please have docstrings for new code? E.g., /* * If the input at the top of the stack(?) is an alias, return the name of * that alias, otherwise return NULL. */ Gave this a quick spin with various aliases and global aliases; behaves as I'd expect. My main concern is that .zshrc files that contain such a bug will not be sourced all the way through. Add a README entry? Thanks, Daniel > +{ > + int flags = inbufflags; > + struct instacks *instackptr = instacktop; > + > + for (;;) > + { > + if (!(flags & INP_CONT)) > + break; > + instackptr--; > + if (instackptr->alias) > + return instackptr->alias->node.nam; > + flags = instackptr->flags; > + } > + > + return NULL; > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2017-01-09 10:33 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-01-07 22:16 Defining function based on alias Peter Stephenson 2017-01-08 19:19 ` Peter Stephenson 2017-01-08 20:14 ` Bart Schaefer 2017-01-08 20:23 ` Peter Stephenson 2017-01-09 0:09 ` Bart Schaefer 2017-01-09 4:02 ` Daniel Shahaf 2017-01-09 10:33 ` Peter Stephenson 2017-01-09 1:55 ` Daniel Shahaf
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).