Re: [RFC][PATCH] Add zrestart()

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

> Subject: [PATCH] Add zrestart()

Nitpick, but the parentheses are wrong.  They imply zrestart() is a C
function, which it isn't.

> +++ b/Doc/Zsh/contrib.yo
> @@ -4649,6 +4649,11 @@ See `Recompiling Functions'
>  ifzman(above)\
>  ifnzman((noderef(Utilities))).
>  )
> +findex(zrestart)
> +item(tt(zrestart))(
> +This function tests whether the shell is able to restart without error and, if 
> +so, restarts the shell.
> +)
> +++ b/Functions/Misc/zrestart
> @@ -0,0 +1,42 @@
> +#
> +# Restarts the shell if and only if it can restart without error.
> +#
> +
> +emulate -LR zsh
> +{
> +  # Some users export $ZDOTDIR, which can mess things up.
> +  local zdotdir=$ZDOTDIR
> +  unset ZDOTDIR

The comment implies those users are doing something unusual, which isn't
the case.  Exporting ZDOTDIR is normal.

More importantly, I don't see any reason to munge $ZDOTDIR in the first
place.  It amounts to second-guessing the user.

> +  print 'Validating...'

Error and progress messages should name their originator.

> +  # Try if the shell can start up without errors. Passing an empty command 
> +  # ensures that the subshell exits immediately after executing all dotfiles.
> +  # We suppress standard out, since we're interested in standard error only.
> +  setopt multios

This option will already be on, and in any case isn't required for the
next line.

> +  local err="$(zsh --interactive --monitor --zle -c '' 2>&1 > /dev/null)"

This doesn't necessarily restart the _same_ zsh, if there's more than
one installed.

Also, I think it's quite a stretch to describe this line as "_tests_
whether the shell is able to restart".  This line executes a whole bunch
of code you have no control over.  I don't think that's an acceptable
approach.