PATCH: vcs_info: Set NO_warn_create_global option

PATCH: vcs_info: Set NO_warn_create_global option

[Frank Terbeck]

I wanted to do the following, because I'd like to have
`check-for-changes' set in my own project directories, but not in
others, I just check out (because it can be slow for big repos):

function estyle-cfc() {
    local d
    local -a cfc_dirs
    cfc_dirs=( ${HOME}/src/code/*(/) )

    for d in ${cfc_dirs}; do
        d=${d%/##}
        [[ $PWD == $d(|/*) ]] && return 0
    done
    return 1
}

zstyle -e ':vcs_info:git:*' \
    check-for-changes 'estyle-cfc && reply=( true ) || reply=( false )'

That however, welcomes me with this nice warning:
VCS_INFO_get_data_git:49: array parameter reply created globally in function

In theory the above should work (and it does except for the warning). It
also works in compsys, where I'm using this:

zstyle -e ':completion:*:approximate:*' \
    max-errors 'reply=( $(( ($#PREFIX + $#SUFFIX) / 3 )) )'

What compsys does is, it disables the warn_create_global option while
it's running.

While I like what the option does (prevent me from cluttering the shell
with all sorts of variables if I forget `local'), I still want to be
able to use `-e' with zstyle.

So, there are three options:
 a) Get the big hammer and just disable `warn_create_global' in
    vcs_info.
 b) Use a smaller hammer, and create a wrapper function around zstyle
    for vcs_info, that disables the option locally.
 c) Disable `warn_create_global' in code executed by "zstyle -e". Or
    make `reply' and `REPLY' special in that case. Or whatever makes the
    most sense.

This patch does what "a)" suggests, basically because it works and I was
lazy. I think "c)" would be the cleanest solution, but I don't know if
that's doable and I haven't looked at the involved code yet.

I might implement "b)" at a later date if "c)" is not feasilbe with
reasonable effort.

Regards, Frank
---
 Functions/VCS_Info/vcs_info |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Functions/VCS_Info/vcs_info b/Functions/VCS_Info/vcs_info
index 906d984..a9e65e1 100644
--- a/Functions/VCS_Info/vcs_info
+++ b/Functions/VCS_Info/vcs_info
@@ -40,7 +40,7 @@ vcs_info_setsys
 # and now, finally create the real vcs_info function
 vcs_info () {
     emulate -L zsh
-    setopt extendedglob
+    setopt extendedglob NO_warn_create_global
 
     [[ -r . ]] || return 0
 
-- 
1.7.0

PATCH: Disable WARN_CREATE_GLOBAL in 'zstyle -e' code

[Frank Terbeck]

[...]
>  c) Disable `warn_create_global' in code executed by "zstyle -e". Or
>     make `reply' and `REPLY' special in that case. Or whatever makes the
>     most sense.
>
> This patch does what "a)" suggests, basically because it works and I was
> lazy. I think "c)" would be the cleanest solution, but I don't know if
> that's doable and I haven't looked at the involved code yet.
[...]

This does "c)". I think this shouldn't break anything. So, I think it's
correct and should be preferable to setting NO_warn_create_global in
vcs_info.

Regards, Frank
---
 Src/Modules/zutil.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index 698b7e3..2d8c61a 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -340,15 +340,22 @@ static char **
 evalstyle(Stypat p)
 {
     int ef = errflag;
+    int reset = 0;
     char **ret, *str;
 
     unsetparam("reply");
+    if (isset(WARNCREATEGLOBAL)) {
+        reset = 1;
+        dosetopt(WARNCREATEGLOBAL, 0, 0);
+    }
     execode(p->eval, 1, 0);
     if (errflag) {
 	errflag = ef;
 	return NULL;
     }
     errflag = ef;
+    if (reset)
+        dosetopt(WARNCREATEGLOBAL, 1, 0);
 
     queue_signals();
     if ((ret = getaparam("reply")))
-- 
1.7.0

PATCH: Disable WARN_CREATE_GLOBAL in 'zstyle -e' code

[Frank Terbeck]

This time with correct indention...

Regards, Frank
---
 Src/Modules/zutil.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/Src/Modules/zutil.c b/Src/Modules/zutil.c
index 698b7e3..443cb26 100644
--- a/Src/Modules/zutil.c
+++ b/Src/Modules/zutil.c
@@ -340,15 +340,22 @@ static char **
 evalstyle(Stypat p)
 {
     int ef = errflag;
+    int reset = 0;
     char **ret, *str;
 
     unsetparam("reply");
+    if (isset(WARNCREATEGLOBAL)) {
+	reset = 1;
+	dosetopt(WARNCREATEGLOBAL, 0, 0);
+    }
     execode(p->eval, 1, 0);
     if (errflag) {
 	errflag = ef;
 	return NULL;
     }
     errflag = ef;
+    if (reset)
+	dosetopt(WARNCREATEGLOBAL, 1, 0);
 
     queue_signals();
     if ((ret = getaparam("reply")))
-- 
1.7.0

Re: PATCH: vcs_info: Set NO_warn_create_global option

[Bart Schaefer]

On Feb 21,  3:53pm, Frank Terbeck wrote:
}
} zstyle -e ':vcs_info:git:*' \
}     check-for-changes 'estyle-cfc && reply=( true ) || reply=( false )'
} 
} That however, welcomes me with this nice warning:
} VCS_INFO_get_data_git:49: array parameter reply created globally in function

} So, there are three options:
}  a) Get the big hammer and just disable `warn_create_global' in
}     vcs_info.
}  b) Use a smaller hammer, and create a wrapper function around zstyle
}     for vcs_info, that disables the option locally.
}  c) Disable `warn_create_global' in code executed by "zstyle -e". Or
}     make `reply' and `REPLY' special in that case. Or whatever makes the
}     most sense.

There's another option

b.5) Write the style like this:

zstyle -e ':vcs_info:git:*' \
    check-for-changes 'typeset -g reply; \
                       estyle-cfc && reply=( true ) || reply=( false )'

Also, rather than (c) I might suggest

d) Suppress warn_create_global for the parameters "reply" and "REPLY"
   at all times.  In fact it probably ought to be suppressed for STTY
   and a few others in the "Parameters Used By The Shell" section of
   the manual.

Re: PATCH: vcs_info: Set NO_warn_create_global option

[Frank Terbeck]

Bart Schaefer wrote:
> On Feb 21,  3:53pm, Frank Terbeck wrote:
>}
>} zstyle -e ':vcs_info:git:*' \
>}     check-for-changes 'estyle-cfc && reply=( true ) || reply=( false )'
>} 
>} That however, welcomes me with this nice warning:
>} VCS_INFO_get_data_git:49: array parameter reply created globally in function
>
>} So, there are three options:
>}  a) Get the big hammer and just disable `warn_create_global' in
>}     vcs_info.
>}  b) Use a smaller hammer, and create a wrapper function around zstyle
>}     for vcs_info, that disables the option locally.
>}  c) Disable `warn_create_global' in code executed by "zstyle -e". Or
>}     make `reply' and `REPLY' special in that case. Or whatever makes the
>}     most sense.
>
> There's another option
>
> b.5) Write the style like this:
>
> zstyle -e ':vcs_info:git:*' \
>     check-for-changes 'typeset -g reply; \
>                        estyle-cfc && reply=( true ) || reply=( false )'

Hm yeah. But I think it would be way more convenient to let the shell do
that automatically, since at least here `reply' is supposed to be used.

> Also, rather than (c) I might suggest
>
> d) Suppress warn_create_global for the parameters "reply" and "REPLY"
>    at all times.  In fact it probably ought to be suppressed for STTY
>    and a few others in the "Parameters Used By The Shell" section of
>    the manual.

I guess, you've seen the patch I set for "c)". That was easy enough.
You're probably right that this should always apply to `reply', `REPLY'
and the like. I don't know how to do that, though. Would you always have
such parameters be available globally, even after an `unset'?

Any ideas where to apply the leverage?

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925

Re: PATCH: vcs_info: Set NO_warn_create_global option

[Bart Schaefer]

On Feb 21,  9:06pm, Frank Terbeck wrote:
}
} > d) Suppress warn_create_global for the parameters "reply" and "REPLY"
} >    at all times.  In fact it probably ought to be suppressed for STTY
} >    and a few others in the "Parameters Used By The Shell" section of
} >    the manual.
} 
} You're probably right that this should always apply to `reply', `REPLY'
} and the like. I don't know how to do that, though. Would you always have
} such parameters be available globally, even after an `unset'?

No, leaving them available would violates some of the other rules of
"non-special" parameters.
 
} Any ideas where to apply the leverage?

In Src/params.c, assignsparam() and assignaparam(), I suppose, by a
check of v->pm->node.nam against a list (which doesn't exist yet) of
parameters that don't require warnings.  I further suppose that list
could itself be controlled by a parameter, but that'd have to be
handled carefully to avoid fetching a value while setting it.

I see PWS has already applied your (a) patch from 27731, so perhaps
this discussion is moot ...

Re: PATCH: vcs_info: Set NO_warn_create_global option

[Frank Terbeck]

Bart Schaefer wrote:
> On Feb 21,  9:06pm, Frank Terbeck wrote:
[...]
> } Any ideas where to apply the leverage?
>
> In Src/params.c, assignsparam() and assignaparam(), I suppose, by a
> check of v->pm->node.nam against a list (which doesn't exist yet) of
> parameters that don't require warnings.  I further suppose that list
> could itself be controlled by a parameter, but that'd have to be
> handled carefully to avoid fetching a value while setting it.

Sounds reasonable, but is well beyond my expertise in zsh's code.

> I see PWS has already applied your (a) patch from 27731, so perhaps
> this discussion is moot ...

Yes I've seen it, too. But I'd like to have `warn_create_global' active
while vcs_info is running (without cutting off useful features such as
zstyle's "-e" option), if possible. It's code should run fine with the
option set. And having it set prevents me from becoming sloppy.

Regards, Frank

-- 
In protocol design, perfection has been reached not when there is
nothing left to add, but when there is nothing left to take away.
                                                  -- RFC 1925