Re: help for writing GNU stow completion

Re: help for writing GNU stow completion

[Daniel Shahaf]

[moving to -workers@ since it's about an outstanding PR]

Aurélien wrote on Sat, 17 Aug 2019 07:51 +00:00:
>   local stow_dir
>   local -a stow_pkg_list
> 
>   eval set -A stow_dir $1
>   [[ -n $stow_dir ]] && stow_pkg_list=( $stow_dir/*(-/N:t) )
> 
>   if [[ ${#stow_pkg_list} -gt 0 ]]; then
>     _values -C "packages from $stow_dir" ${stow_pkg_list[@]}
>   else
>     _message "no packages found in $stow_dir"
>   fi
> 
> and the completion of parameters such as'$HOME' or'~/' works well !
> 
> I pushed my modifications on github. Thank you for the answers :-)

Sorry, but I have to object to the PR as it stands.  As I said, using
«eval $1» causes expressions on the command line to be evaluated
_when completion is attempted_.  To me, that breaks the principle of
least surprise, and could lead to unexpected and undesired results.

I would strongly prefer another solution, or to be corrected on my
interpretation that this violates least surprise.

Also, I don't think it's ideal to have «foo --opt=$ARG<TAB>» work and
«bar --opt=$ARG<TAB>» not work.

Cheers,

Daniel

Re: help for writing GNU stow completion

[dana]

On 17 Aug 2019, at 12:53, Daniel Shahaf <d.s@daniel.shahaf.name> wrote:
> Sorry, but I have to object to the PR as it stands.  As I said, using
> «eval $1» causes expressions on the command line to be evaluated
> _when completion is attempted_.  To me, that breaks the principle of
> least surprise, and could lead to unexpected and undesired results.
>
> I would strongly prefer another solution, or to be corrected on my
> interpretation that this violates least surprise.

Oh, i didn't notice that the GitHub thing was a PR for zsh.

I agree with this. And i can't think of a general-purpose alternative that's
currently built into the shell.

Apparently i made the same mistake when i wrote _composer, though; attached
replaces the eval by (as i mentioned) what seems to be the prevailing
convention. I think a similar change might be warranted in _git @ 2866?

dana


diff --git a/Completion/Unix/Command/_composer b/Completion/Unix/Command/_composer
index 2b9f2cd32..191350453 100644
--- a/Completion/Unix/Command/_composer
+++ b/Completion/Unix/Command/_composer
@@ -115,7 +115,7 @@ __composer_prune_global_opts() {
 (( $+functions[__composer_update_work_dir] )) ||
 __composer_update_work_dir() {
   if [[ -n ${(v)opt_args[(i)(-d|--working-dir)]} ]]; then
-    eval _composer_work_dir=${(v)opt_args[(i)(-d|--working-dir)]}
+    _composer_work_dir=${(Q)${(v)opt_args[(i)(-d|--working-dir)]}}
   elif [[ -z $_composer_work_dir ]]; then
     _composer_work_dir=$PWD
   fi

Re: help for writing GNU stow completion

[Aurélien]

Le 17/08/2019 ?? 19:53, Daniel Shahaf a ??crit??:
> [moving to -workers@ since it's about an outstanding PR]
> 
> Aur??lien wrote on Sat, 17 Aug 2019 07:51 +00:00:
>>   local stow_dir
>>   local -a stow_pkg_list
>>
>>   eval set -A stow_dir $1
>>   [[ -n $stow_dir ]] && stow_pkg_list=( $stow_dir/*(-/N:t) )
>>
>>   if [[ ${#stow_pkg_list} -gt 0 ]]; then
>>     _values -C "packages from $stow_dir" ${stow_pkg_list[@]}
>>   else
>>     _message "no packages found in $stow_dir"
>>   fi
>>
>> and the completion of parameters such as'$HOME' or'~/' works well !
>>
>> I pushed my modifications on github. Thank you for the answers :-)
> 
> Sorry, but I have to object to the PR as it stands.  As I said, using
> ??eval $1?? causes expressions on the command line to be evaluated
> _when completion is attempted_.  To me, that breaks the principle of
> least surprise, and could lead to unexpected and undesired results.
> 

Okay, I understand the idea, so I tried to change the function to this:

    __stow_complete_packages() {
        local stow_dir=${(Q)1}
        local -a stow_pkg_list=( $stow_dir/*(-/N:t) )
        if [[ ${#stow_pkg_list} -gt 0 ]]; then
        _values -C "packages from $stow_dir" ${stow_pkg_list[@]}
        else
        _message "no packages found in $stow_dir"
        fi
    }

But now if I do ?? stow --dir=$HOME/.dotfiles<TAB> ??, I get ????no packages
found in $HOME/.dotfiles????. Maybe I missed something???

> I would strongly prefer another solution, or to be corrected on my
> interpretation that this violates least surprise.
> 
> Also, I don't think it's ideal to have ??foo --opt=$ARG<TAB>?? work and
> ??bar --opt=$ARG<TAB>?? not work.
> 

To remove any ambiguity, I want to complete the stow packages

    stow --dir $HOME/.dotfiles <TAB>
    _packages from /home/username/.dotfiles_
    git   myrepos   nano   nvim   ranger   rofi   [???]

And not the directories

    stow --dir $HOME/.dotfiles/<TAB>
    _directory_
    git   myrepos   nano   nvim   ranger   rofi   [???]

My initial goal was to get a completion of the same type as what
password-store offers (see: line 127 at
https://git.zx2c4.com/password-store/tree/src/completion/pass.zsh-completion).
This is where the ????find???? in the first version of the script comes from.

Naively, as a user, when I do ????stow --dir $HOME/.dotfiles <TAB>???? and
the directory in question contains many subdirectories, if the
completion does not offer any packages, it is surprising.

Now, due to my limited experience in writing completion for zsh, I don't
know what would be the least bad of the solutions.

Thank you for your time.

-- 
Aur??lien

Re: help for writing GNU stow completion

[dana]

(idk what's up with the ???? here, it says UTF-8 but it's all messed up)

On 20 Aug 2019, at 12:45, Aurélien <orel_jf@yahoo.fr> wrote:
> But now if I do ?? stow --dir=$HOME/.dotfiles<TAB> ??, I get ????no packages
> found in $HOME/.dotfiles????. Maybe I missed something???

It's because of what i said before — the argument you're passing to that
helper function is the raw, unevaluated text from the command line, so it
isn't expanding $HOME. The only easy way i can think of to *make* a completion
function expand that is to evaluate it, which you can do with eval or, as i
forgot to mention before, the (e) expansion flag. But if you do that, there is
a chance (though probably not a huge one) that you might trigger unexpected
and potentially even destructive side-effects when the user invokes
completion, which is why Daniel objected to it.

If there were some kind of 'safe eval' that just performed 'read-only'
parameter expansions, you could use that, but as far as i know there isn't
currently anything like that that's built into the shell, and it's not ideal
to special-case this one function.

So, without a general solution, the more limited version in your last message
might be the best option for now

dana

Re: help for writing GNU stow completion

[Aurélien]

Le 25/08/2019 à 03:14, dana a écrit :
> 
> So, without a general solution, the more limited version in your last message
> might be the best option for now
> 

Hello,

First of all, I'm sorry for the long response delay.

Following these exchanges, I modified the pull request to come back to a
version that does not use "eval" (see:
https://github.com/zsh-users/zsh/pull/36).

I hope that this work could be integrated :-)

-- 
Aurélien

Re: help for writing GNU stow completion

[dana]

On 13 Oct 2019, at 12:51, Aurélien <orel_jf@yahoo.fr> wrote:
> Following these exchanges, I modified the pull request to come back to a
> version that does not use "eval" (see:
> https://github.com/zsh-users/zsh/pull/36).

I think your most recent change looks OK, as far as the evaluation stuff is
concerned. I noticed some other minor issues, though. Not actually sure what
the etiquette is here, but since the original patch was done on GitHub i've
left my comments there

dana