Fix running in shells with `-u`/`-o nounset` option set

[ntninja]

Maybe there are more issue, but these are the ones I found when preparing my latest set of patches for oh-my-zsh

(Original submission to OMZ)

[agkozak]

Thank you so much! I'll look over this tomorrow.

[agkozak]

First, let me wholeheartedly thank you for bringing this issue to my attention. (Incidentally, I love this sort of thing).

I agree that Zsh-z should work without error when the user has chosen to enable setopt NO_UNSET. Overall, your approach works (although, as you foresaw, I think I found some more variables that need to be accounted for).

I would propose a different approach, though. I think that I and other potential contributors in the future are unlikely to adopt a "careful-about-NO_UNSET" coding style; it is probable that people would just keep introducing potentially unset variables into the code, and we would have to fix them from time to time. Instead of mandating an idiosyncratic coding style for the project, and to make the solution future-proof, what I would do is to rewrite a few lines of code outside functions, and then to enable setopt LOCAL_OPTIONS UNSET inside functions:

diff --git a/zsh-z.plugin.zsh b/zsh-z.plugin.zsh
index 3ffaed5..e6792ad 100644
--- a/zsh-z.plugin.zsh
+++ b/zsh-z.plugin.zsh
@@ -102,13 +102,13 @@ With no ARGUMENT, list the directory history in ascending rank.
 (( $+EPOCHSECONDS )) || zmodload zsh/datetime

 # Load zsh/files, if necessary
-[[ ${builtins[zf_chown]} == 'defined' &&
-   ${builtins[zf_mv]}    == 'defined' &&
-   ${builtins[zf_rm]}    == 'defined' ]] ||
+[[ ${builtins[zf_chown]:-} == 'defined' &&
+   ${builtins[zf_mv]:-}    == 'defined' &&
+   ${builtins[zf_rm]:-}    == 'defined' ]] ||
   zmodload -F zsh/files b:zf_chown b:zf_mv b:zf_rm

 # Load zsh/system, if necessary
-[[ ${modules[zsh/system]} == 'loaded' ]] || zmodload zsh/system &> /dev/null
+[[ ${modules[zsh/system]:-} == 'loaded' ]] || zmodload zsh/system &> /dev/null

 # Global associative array for internal use
 typeset -gA ZSHZ
@@ -143,7 +143,7 @@ is-at-least 5.3.0 && ZSHZ[PRINTV]=1
 zshz() {

   # Don't use `emulate -L zsh' - it breaks PUSHD_IGNORE_DUPS
-  setopt LOCAL_OPTIONS NO_KSH_ARRAYS NO_SH_WORD_SPLIT EXTENDED_GLOB
+  setopt LOCAL_OPTIONS NO_KSH_ARRAYS NO_SH_WORD_SPLIT EXTENDED_GLOB UNSET
   (( ZSHZ_DEBUG )) && setopt LOCAL_OPTIONS WARN_CREATE_GLOBAL

   local REPLY
@@ -862,6 +862,9 @@ alias ${ZSHZ_CMD:-${_Z_CMD:-z}}='zshz 2>&1'
 #   ZSHZ
 ############################################################
 _zshz_precmd() {
+  # Protect against `setopt NO_UNSET'
+  setopt LOCAL_OPTIONS UNSET
+
   # Do not add PWD to datafile when in HOME directory, or
   # if `z -x' has just been run
   [[ $PWD == "$HOME" ]] || (( ZSHZ[DIRECTORY_REMOVED] )) && return
@@ -936,7 +939,7 @@ ZSHZ[FUNCTIONS]='_zshz_usage
 # Enable WARN_NESTED_VAR for functions listed in
 #   ZSHZ[FUNCTIONS]
 ############################################################
-(( ZSHZ_DEBUG )) && () {
+(( ${+ZSHZ_DEBUG} )) && () {
   if is-at-least 5.4.0; then
     local x
     for x in ${=ZSHZ[FUNCTIONS]}; do

(I threw that solution together rather quickly, so I may have missed something.)

Let me know what you think about this suggestion.

[agkozak]

(I originally wrote "and then to enable setopt LOCAL_OPTIONS NO_UNSET inside functions"; of course, I meant setopt LOCAL_OPTIONS UNSET , the idea being to enable us to adopt a more mainstream coding style within each function.)

[agkozak]

OK, I've made the simpler changes that I proposed to get the plugin working with setopt NO_UNSET. I might consider changing my code style to be stricter in the future; for now, I just want to make sure that the plugin will not break.

Thank you for bringing this issue to my attention. I have credited you in the documentation.