From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19979 invoked by alias); 1 Aug 2016 20:26:34 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 38985 Received: (qmail 21258 invoked from network); 1 Aug 2016 20:26:34 -0000 X-Qmail-Scanner-Diagnostics: from nm34.bullet.mail.ir2.yahoo.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(212.82.96.59):SA:0(0.0/5.0):. Processed in 0.218536 secs); 01 Aug 2016 20:26:34 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=FREEMAIL_FROM,SPF_PASS, T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: okiddle@yahoo.co.uk X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: pass (ns1.primenet.com.au: SPF record at _spf.mail.yahoo.com designates 212.82.96.59 as permitted sender) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.co.uk; s=s2048; t=1470082776; bh=3SkHc2c6bV1xEB729XPQcP3imEuqcshqY8bTwdJqWAU=; h=cc:In-reply-to:From:References:To:Subject:Date:From:Subject; b=bDr31pVrxMg/Ez/e6Du1IobuSAGbpT8WdlQPZ+8CS8cTUeWM3FYNPSB2POdQuU6o50EE24dO5uNaqnHOwW6vv+9jxBZ1k57hqFRmtMMUVUvuUHeSpP+wR2zCcAubNX1GPcRj1u7SUKrj1V2kVha6SUap6foq96DPPMVclN+cUhUYxcAzRsS2XelobQc2ROPUaeAMLprbsQ5+YP2mKtxlJDCLvnZAwK/g6wXWsLLo/TyEgshtVHPQzGryXNh8xKddv3vDnozWiijbOMJYitGHdF57xawL5CEEu4IkG61q19BYcxaUklTZDQOMfauXU3vRUdZ5Q0dBv9VAtPdjPG/JIQ== X-Yahoo-Newman-Id: 255046.32870.bm@smtp133.mail.ir2.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: wNczP0oVM1nnAOQwgT5UYxFKppa13qlPznoZ5uMHYzFV9va cVPJ17e7tSpiFS8pIfRDHRgOdcwccut3pu3q1MxgcjqObUgKkUjDGBaQhSnp U52huW7PxpRuZ3l3b2n5cRPswMNAKwJqzKlpXQQsRn6CT1gQ2EvYkb.qBXD. qbgiydcBbXvbyCrXZQOHtC.1V6IJTuT9G7GlGp2VsKZvrDWw9qDfPlntc9_P nyu8AXxGnv_UMTl_y6tB6Qd7fbNXsepUMObbUwk9vYzK5zY2CB6R3qrG_9mK 1tBp6HjiZRmlwzEBDhWTCqal6LJswFTW74GC5Ks_UKAJYhWAeCTOg4wOtBZL _sa9bHN90L_ZVIxB2sgJIdqYOu6ZINnatvv29cCy7yLsEV43kcOxndWc3S5k c0X3TrKBHynAvhB6_53d2.HCwz5LjrXP5ISdY86sAVOVu7V.SKytbXCX4vJL b1krhjETq7kSGTSPzzQCgGI7ed52PxftaMqSmg9wnIAsCfsWu7I9xHu4gnnW SWHTd59201kubD7Wa8UgAEFCiQtpfoAlRPU6Z79wKf9A- X-Yahoo-SMTP: opAkk_CswBAce_kJ3nIPlH80cJI- cc: zsh-workers@zsh.org In-reply-to: <20160731110516.GA20521@pileus.org> From: Oliver Kiddle References: <20160731110516.GA20521@pileus.org> To: Andy Spencer Subject: Re: [PATCH] Add completion for cpupower command MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-ID: <16721.1470082775.1@hydra.kiddle.eu> Date: Mon, 01 Aug 2016 22:19:35 +0200 Message-ID: <16722.1470082775@hydra.kiddle.eu> Andy Spencer wrote: > These are mostly static options with the exception of the frequency-set > arguments for the governor and min/max frequencies, which are pulled > from the output of frequency-info. Thanks for contributing this. I've taken a look and have a few comments. Mostly all minor points. > +#compdef cpupower > + > +local -a ret state cmds args expl context line ret should be initialised to 1 and shouldn't be of array type. In this function, I'd recommand having _arguments update $curcontext instead of using the context array. So pass -C to _arguments and use: local curcontext="$curcontext" The array form with $context is only necessary if more than one state is simultaneously possible. That happens when you have optional arguments which cpupower doesn't. > + > +_arguments \ > + '(-h --help)'{-h,--help}'[print usage information]' \ > + '(-v --version)'{-v,--version}'[print package name and version]' \ As is often the case with version and help options, no other options are valid following them. So these two exclusion lists can be reduced to (- :) e.g: '(- :)'{-h,--help}'[print usage information]' Also, doesn't cpupower also have a -c/--cpu option for selecting specific CPUs and -d/--debug? > + ':cmd:->cmds' \ > + '*::arg:->args' && ret=0 > + > +case $state in > + cmds) It'd also be useful to complete these commands after the help command but this doesn't do that. > + args) > + args=( ) > + case ${words[1]} in With commands that take subcommands, it is common practice to insert the subcommand in to the command component of $curcontext at this point. Something like: curcontext="${curcontext%:*}-$words[1]:" > + frequency-info) > + args+=( > + '(-e --debug)'{-e,--debug}'[print debug info]' > + '(-f --freq)'{-f,--freq}'[show current frequency]' > + '(-w --hwfreq)'{-w,--hwfreq}'[show current hardware frequency]' > + '(-l --hwlimits)'{-l,--hwlimits}'[show min/max frequency allowed]' > + '(-d --driver)'{-d,--driver}'[show the kernel driver in use]' > + '(-p --policy)'{-p,--policy}'[show the current cpufreq policy]' > + '(-g --governors)'{-g,--governors}'[show available governers]' > + '(-r --related-cpus)'{-a,--related-cpus}'[show cpus that run at the same frequency]' > + '(-a --affected-cpus)'{-a,--affected-cpus}'[show software controlled cpus]' > + '(-s --stats)'{-s,--stats}'[show cpufreq statistics]' > + '(-y --latency)'{-y,--latency}'[show frequency change latency]' > + '(-o --proc)'{-o,--proc}'[print old style proc info]' > + '(-m --human)'{-m,--human}'[use human readable output]' > + '(-n --no-rounding)'{-n,--no-rounding}'[disable rounding of values]' > + ) The exclusion lists could be a lot more restrictive. Most combinations print: You can't specify more than one --cpu parameter and/or more than one output-specific argument I think it is perhaps only the last three options that can be used with others. > + frequency-set) > + '(-f --freq)'{-f,--freq}'[new frequency for userspace governor]:freq->freq' There's a colon missing on that line. > + args+=( > + '(-d --disable)'{-d,--disable}'[disable specific sleep state]:stateno' > + '(-e --enable)'{-e,--enable}'[enable specific sleep state]:stateno' Not especially important but it's not possible to complete those states is it? Documentation points to /sys/devices/system/cpu/cpu*/cpuidle/state*. Those don't exist on my system but that might be because it's very old. > + monitor) > + '-i[mesurement interval]:secs' measurement > + _arguments "${args[@]}" && ret=0 You can pass -s to this _arguments as it seems cpupower lets you clump together options, e.g. cpupower frequency-info -fn > + case $state in > + freq) > + compadd $(cpupower frequency-info | > + sed -n 's/ //g; s/,/ /g; s/availablefrequencysteps://p') It'd be nice to have descriptions for this and the governors. e.g.: _wanted frequencies expl frequency compadd .... And if you stick to the context array and no -C option to _arguments, this would need to be: _wanted -C "$context" frequencies ... Oliver