* Re: help for writing GNU stow completion [not found] ` <d30637f3-cbbb-8b93-35f2-421a1a99e1dc@yahoo.fr> @ 2019-08-17 17:53 ` Daniel Shahaf 2019-08-17 21:44 ` dana 2019-08-20 17:45 ` Aurélien 0 siblings, 2 replies; 6+ messages in thread From: Daniel Shahaf @ 2019-08-17 17:53 UTC (permalink / raw) To: Aurélien, dana; +Cc: zsh-users [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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: help for writing GNU stow completion 2019-08-17 17:53 ` help for writing GNU stow completion Daniel Shahaf @ 2019-08-17 21:44 ` dana 2019-08-20 17:45 ` Aurélien 1 sibling, 0 replies; 6+ messages in thread From: dana @ 2019-08-17 21:44 UTC (permalink / raw) To: Daniel Shahaf; +Cc: Aurélien, Zsh hackers list 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: help for writing GNU stow completion 2019-08-17 17:53 ` help for writing GNU stow completion Daniel Shahaf 2019-08-17 21:44 ` dana @ 2019-08-20 17:45 ` Aurélien 2019-08-25 1:14 ` dana 1 sibling, 1 reply; 6+ messages in thread From: Aurélien @ 2019-08-20 17:45 UTC (permalink / raw) To: Daniel Shahaf, dana; +Cc: zsh-users 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: help for writing GNU stow completion 2019-08-20 17:45 ` Aurélien @ 2019-08-25 1:14 ` dana 2019-10-13 17:51 ` Aurélien 0 siblings, 1 reply; 6+ messages in thread From: dana @ 2019-08-25 1:14 UTC (permalink / raw) To: Aurélien; +Cc: Daniel Shahaf, Zsh hackers list (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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: help for writing GNU stow completion 2019-08-25 1:14 ` dana @ 2019-10-13 17:51 ` Aurélien 2019-10-16 1:14 ` dana 0 siblings, 1 reply; 6+ messages in thread From: Aurélien @ 2019-10-13 17:51 UTC (permalink / raw) To: Zsh hackers list; +Cc: dana, Daniel Shahaf 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: help for writing GNU stow completion 2019-10-13 17:51 ` Aurélien @ 2019-10-16 1:14 ` dana 0 siblings, 0 replies; 6+ messages in thread From: dana @ 2019-10-16 1:14 UTC (permalink / raw) To: Aurélien; +Cc: Zsh hackers list, Daniel Shahaf 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-16 1:14 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1e5195bb-3126-8d0c-8a6a-1f5a5fd2a6c0@yahoo.fr> [not found] ` <BAE67462-BF39-481A-BCD6-DE288FE6CC92@dana.is> [not found] ` <d30637f3-cbbb-8b93-35f2-421a1a99e1dc@yahoo.fr> 2019-08-17 17:53 ` help for writing GNU stow completion Daniel Shahaf 2019-08-17 21:44 ` dana 2019-08-20 17:45 ` Aurélien 2019-08-25 1:14 ` dana 2019-10-13 17:51 ` Aurélien 2019-10-16 1:14 ` dana
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).