* [PATCH 0/1] *** Add luarocks completion ***
@ 2018-05-26 16:14 doron.behar
2018-05-26 16:14 ` [PATCH 1/1] Squashed commit of the following: doron.behar
0 siblings, 1 reply; 14+ messages in thread
From: doron.behar @ 2018-05-26 16:14 UTC (permalink / raw)
To: zsh-workers
From: Doron Behar <doron.behar@gmail.com>
*** Add luarocks completion ***
Doron Behar (1):
Squashed commit of the following:
Completion/Unix/Command/_luarocks | 560 ++++++++++++++++++++++++++++++
1 file changed, 560 insertions(+)
create mode 100644 Completion/Unix/Command/_luarocks
--
2.17.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/1] Squashed commit of the following:
2018-05-26 16:14 [PATCH 0/1] *** Add luarocks completion *** doron.behar
@ 2018-05-26 16:14 ` doron.behar
2018-05-28 10:48 ` Oliver Kiddle
0 siblings, 1 reply; 14+ messages in thread
From: doron.behar @ 2018-05-26 16:14 UTC (permalink / raw)
To: zsh-workers
From: Doron Behar <doron.behar@gmail.com>
commit bc61113afd451d11f07c1c44dafb992cc134a960
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat May 26 17:37:20 2018 +0300
Consider `--tree` in versions completion.
commit 8c727a34edb3f892bbf75427d15fa71f62344b3e
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat May 26 17:30:59 2018 +0300
Consider `--tree` when searching installed rocks.
commit c9eb6832e617fddb5aafc26b55cd30630a18959e
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat May 26 11:12:41 2018 +0300
Improve `___luarocks_installed_rocks_cache_policy`.
Handle multiple trees when evaluating installed rocks cache validity -
both system wide and user's.
commit 78d444d0393e9da28ccbb783c88c22d6e784352d
Author: Doron Behar <doron.behar@gmail.com>
Date: Mon May 14 23:51:56 2018 +0300
Use +functions[] for all helpers.
commit 27a5a8e9bb0b2fbafc3c6a4d7fb2928b4cd6d927
Author: Doron Behar <doron.behar@gmail.com>
Date: Mon May 14 23:34:51 2018 +0300
Use 2 spaces instead of tabs.
commit 063240718e07154af4d0242a6611ea8fcfd4bffd
Author: Doron Behar <doron.behar@gmail.com>
Date: Mon Apr 30 18:54:02 2018 +0300
Use a generic sub command completer.
commit 12310709ab3c1d4fad464355f10e32ec44193777
Author: Doron Behar <doron.behar@gmail.com>
Date: Mon Apr 30 13:48:39 2018 +0300
Fix git tag completion by autoloading _git
commit 7e71ce226dc18bfb3e72610d33633ff20857f8c4
Author: Doron Behar <doron.behar@gmail.com>
Date: Mon Apr 30 13:48:03 2018 +0300
Make cache policy function safer.
Add TODO for using zstyle for manifest files location when checking when
they were last modified.
commit 8b6aafc444fa128ca2dce277cc3a1579788446a2
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 12:06:49 2018 +0300
Write a better comment for last TODO.
I don't know how to finish it.
commit fc48b1583366a672a2ec011029505e3b4a3e7711
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 12:05:08 2018 +0300
Finish completions for purge and new_version.
commit 2791ee20ccc6ff35817f43baad0d311c0026bab1
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 11:15:48 2018 +0300
Expand __luarocks_rock_version so it accpets args.
Write `_luarocks_write_rockspec`.
commit f16c723a4fa5f5a9e07cb75a62eaeeea17b472c1
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 10:22:30 2018 +0300
Finish `_luarocks_doc` and `_luarocks_config`.
commit 783783b76d149b47adecf0d11bb7db122477f51b
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 10:21:07 2018 +0300
General cleanup.
commit 2e4771d6fd5937069ee25fe6edb92c6b7d6afaed
Author: Doron Behar <doron.behar@gmail.com>
Date: Sat Apr 28 10:20:32 2018 +0300
Finish helper `__luarocks_lua_versions`.
commit 5daeade7690b04d7f102e5b144a6ba85fd3ee7d3
Author: Doron Behar <doron.behar@gmail.com>
Date: Fri Apr 27 23:11:54 2018 +0300
General internal conventions sync.
commit e29115c07ee8cecfa4ddaeb2f8e09a1bc0ae9e74
Author: Doron Behar <doron.behar@gmail.com>
Date: Fri Apr 27 22:56:59 2018 +0300
Write all simple sub commands completions.
Most importantly, write (using `_cache_` functions) helpers that
complete installed rocks with their descriptions generated with
`luarocks show <>`.
commit 6c35e70c9af278621a2bc3067358071d0185a08d
Author: Doron Behar <doron.behar@gmail.com>
Date: Fri Apr 27 22:52:50 2018 +0300
Make *all* helpers functions begin with __luarocks.
commit 45a71b968e0e6b60503a3d1e3ffb9bad0ab79500
Author: Doron Behar <doron.behar@gmail.com>
Date: Thu Apr 26 12:48:18 2018 +0300
Add helpers section.
commit 751eed19b4e0fb787d28f1a9ea996c296341200d
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 21:25:47 2018 +0300
Write better sub commands comments.
commit 6a47db186ceaf74e6bb1ac6aaa7cef602ecfd3bc
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 19:40:07 2018 +0300
Use better naming scheme for common helpers.
Rename `_luarocks_build_deps_mode` to `__luarocks_deps_mode`.
Rename `_luarocks_version` to `__luarocks_version`.
commit 92164093db3f4df6e8b3cdd40e218998c169b2ed
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 19:32:54 2018 +0300
Add curcontext case for every subcommand.
Thanks `src/_android`!
commit dd42a18a26fb54ccab00646b6becede2a405036a
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 19:00:13 2018 +0300
Remove variables and use their contents directly.
commit 11b039b124b7fa2bdda9b419f29f4766f121f25f
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 15:27:31 2018 +0300
Add marker style comments.
commit 60f9055538ee1063e7139c084ebb9dd504a73346
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 15:02:22 2018 +0300
Remove architecture related option completion.
commit 78f677f2d7a19e50b68a6cf12e4b297aa64f4a95
Author: Doron Behar <doron.behar@gmail.com>
Date: Wed Apr 25 13:02:40 2018 +0300
Add variables for all commands and options.
---
Completion/Unix/Command/_luarocks | 560 ++++++++++++++++++++++++++++++
1 file changed, 560 insertions(+)
create mode 100644 Completion/Unix/Command/_luarocks
diff --git a/Completion/Unix/Command/_luarocks b/Completion/Unix/Command/_luarocks
new file mode 100644
index 000000000..a02bd06b5
--- /dev/null
+++ b/Completion/Unix/Command/_luarocks
@@ -0,0 +1,560 @@
+#compdef luarocks
+
+# {{{ helper: luarocks commands
+(( $+functions[__luarocks_command] )) ||
+__luarocks_command(){
+ local -a commands=(
+ build:'Build/compile a rock'
+ config:'Query information about the LuaRocks configuration'
+ doc:'Show documentation for an installed rock'
+ download:'Download a specific rock file from a rocks server'
+ help:'Help on commands'
+ install:'Install a rock'
+ lint:'Check syntax of a rockspec'
+ list:'List currently installed rocks'
+ make:'Compile package in current directory using a rockspec'
+ new_version:'Auto-write a rockspec for a new version of a rock'
+ pack:'Create a rock, packing sources or binaries'
+ path:'Return the currently configured package path'
+ purge:'Remove all installed rocks from a tree'
+ remove:'Uninstall a rock'
+ search:'Query the LuaRocks servers'
+ show:'Show information about an installed rock'
+ unpack:'Unpack the contents of a rock'
+ upload:'Upload a rockspec to the public rocks repository'
+ write_rockspec:'Write a template for a rockspec file'
+ )
+ _describe -t commands 'command' commands "$@"
+}
+# }}}
+# {{{ helper: dependencies mode
+local option_deps_mode='--deps-mode=[How to handle dependencies]:MODE:__luarocks_deps_mode'
+(( $+functions[__luarocks_deps_mode] )) ||
+__luarocks_deps_mode(){
+ local modes=(
+ 'all:use all trees from the rocks_trees list for finding dependencies'
+ 'one:use only the current tree (possibly set with --tree)'
+ 'order:use trees based on order (use the current tree and all trees below it on the rocks_trees list)'
+ 'none:ignore dependencies altogether'
+ )
+ _describe 'dependencies mode' modes
+}
+# }}}
+# {{{ helper: versions of an external rock or nothing for rockspec file
+(( $+functions[__luarocks_rock_version] )) ||
+__luarocks_rock_version(){
+ local i=2
+ while [[ -n "${words[$i]}" ]]; do
+ if [[ ! "${words[$i]}" =~ '^-' ]]; then
+ case "$1" in
+ "external_or_local")
+ if [[ ! -f "${words[$i]}" ]]; then
+ _message -e "version for external rock ${words[$i]}"
+ return
+ else
+ _message -e "version for local rock ${words[$i]}"
+ fi
+ ;;
+ "installed")
+ # TODO: actually complete versions of installed rocks using the cache
+ # How does luarocks handles multiple versions of the same package?
+ # If anybody knows, please write something beautiful here
+ tree="$2"
+ if [[ -z "${tree}" ]]; then
+ _message -e "version for installed rock ${words[$i]}"
+ else
+ _message -e "version for installed rock ${words[$i]} on tree ${tree}"
+ fi
+ return
+ ;;
+ "new_version")
+ if [[ -f "${words[$i]}" ]]; then
+ _message -e "new version for rock ${words[$i]}"
+ return
+ fi
+ ;;
+ "new_rock")
+ _message -e "version for new rock ${words[$i]}"
+ return
+ ;;
+ esac
+ fi
+ i=$(( i + 1 ))
+ done
+}
+# }}}
+# {{{ helper: lua versions
+(( $+functions[__luarocks_lua_versions] )) ||
+__luarocks_lua_versions(){
+ _values -s , 5.3 5.2 5.1
+}
+# }}}
+# {{{ helper: installed rocks cache policy
+(( $+functions[___luarocks_installed_rocks_cache_policy] )) ||
+___luarocks_installed_rocks_cache_policy(){
+ # the number of seconds since 1970-01-01 the manifest files were modified
+ local user_manifest_last_date_modified="$(date -r ${HOME}/.luarocks/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
+ local system_manifest_last_date_modified="$(date -r /usr/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
+ # the number of seconds since 1970-01-01 the cache file was modified
+ local cache_last_date_modified="$(date -r $1 +%s 2>/dev/null)"
+ if [[ ! -z ${cache_last_date_modified} && ! -z ${user_manifest_last_date_modified} && ! -z ${system_manifest_last_date_modified} ]]; then
+ # if the manifest file is newer then the cache:
+ if [ ${user_manifest_last_date_modified} -ge ${cache_last_date_modified} ] || [ ${system_manifest_last_date_modified} -ge ${cache_last_date_modified} ]; then
+ (( 1 ))
+ else
+ (( 0 ))
+ fi
+ else
+ (( 1 ))
+ fi
+}
+# }}}
+# {{{ helper: installed rocks
+(( $+functions[__luarocks_installed_rocks] )) ||
+__luarocks_installed_rocks(){
+ # This function optionally recieves one argument of the tree in which
+ # installed rocks are searched for. If this argument is used, the installed
+ # rocks which will be completed by this function will not use the cache which
+ # is valid only for installed rocks on default trees like /usr/lib/luarocks
+ # and ~/.luarocks
+ local tree="$1"
+ if [[ -z ${tree} ]]; then
+ local update_policy
+ zstyle -s ":completion:${curcontext}:" cache-policy update_policy
+ if [[ -z "$update_policy" ]]; then
+ zstyle ":completion:${curcontext}:" cache-policy ___luarocks_installed_rocks_cache_policy
+ fi
+ if _cache_invalid luarocks_installed_list; then
+ rocks_list=($(luarocks list --porcelain))
+ _store_cache luarocks_installed_list rocks_list
+ else
+ _retrieve_cache luarocks_installed_list
+ fi
+ if [[ -z ${rocks_list} ]]; then
+ _message -r "no installed rocks"
+ return
+ fi
+ if _cache_invalid luarocks_installed_names; then
+ rocks_names=()
+ for i in {1.."${#rocks_list[@]}"..4}; do
+ rocks_names+=("${rocks_list[$i]}")
+ done
+ _store_cache luarocks_installed_names rocks_names
+ else
+ _retrieve_cache luarocks_installed_names
+ fi
+ if _cache_invalid luarocks_installed_versions; then
+ rocks_versions=()
+ for i in {2.."${#rocks_list[@]}"..4}; do
+ rocks_versions+=("${rocks_list[$i]}")
+ done
+ _store_cache luarocks_installed_versions rocks_versions
+ else
+ _retrieve_cache luarocks_installed_versions
+ fi
+ if _cache_invalid luarocks_installed_descriptions; then
+ rocks_descriptions=()
+ for i in {1.."${#rocks_names[@]}"}; do
+ name_version_description="$(luarocks show ${rocks_names[$i]} 2>/dev/null | head -2 | tail -1)"
+ total_length=${#name_version_description}
+ garbage_length="$((${#rocks_names[$i]} + ${#rocks_versions[$i]} + 5))"
+ description="${name_version_description[${garbage_length},${total_length}]}"
+ rocks_descriptions+=("${description}")
+ done
+ _store_cache luarocks_installed_descriptions rocks_descriptions
+ else
+ _retrieve_cache luarocks_installed_descriptions
+ fi
+ if _cache_invalid luarocks_installed_names_and_descriptions; then
+ rocks_names_and_descriptions=()
+ for i in {1.."${#rocks_names[@]}"}; do
+ name_and_description=${rocks_names[$i]}:${rocks_descriptions[$i]}
+ rocks_names_and_descriptions+=(${name_and_description})
+ done
+ else
+ _store_cache luarocks_installed_names_and_descriptions rocks_names_and_descriptions
+ fi
+ else
+ rocks_list=($(luarocks --tree="${tree}" list --porcelain 2> /dev/null))
+ if [[ -z ${rocks_list} ]]; then
+ _message "no installed rocks in the specified tree"
+ return
+ fi
+ rocks_names=()
+ for i in {1.."${#rocks_list[@]}"..4}; do
+ rocks_names+=("${rocks_list[$i]}")
+ done
+ rocks_versions=()
+ for i in {2.."${#rocks_list[@]}"..4}; do
+ rocks_versions+=("${rocks_list[$i]}")
+ done
+ rocks_descriptions=()
+ for i in {1.."${#rocks_names[@]}"}; do
+ name_version_description="$(luarocks show ${rocks_names[$i]} 2> /dev/null | head -2 | tail -1)"
+ total_length=${#name_version_description}
+ garbage_length="$((${#rocks_names[$i]} + ${#rocks_versions[$i]} + 5))"
+ description="${name_version_description[${garbage_length},${total_length}]}"
+ rocks_descriptions+=("${description}")
+ done
+ rocks_names_and_descriptions=()
+ for i in {1.."${#rocks_names[@]}"}; do
+ name_and_description=${rocks_names[$i]}:${rocks_descriptions[$i]}
+ rocks_names_and_descriptions+=(${name_and_description})
+ done
+ fi
+ _describe 'installed rocks' rocks_names_and_descriptions
+}
+# }}}
+# {{{ helper: rocks wrapper
+# Used to complete one or more of the followings:
+# - .rockspec file
+# - .src.rock file
+# - external rock
+(( $+functions[__luarocks_rock] )) ||
+__luarocks_rock(){
+ local -a alts=()
+ while [[ $# -gt 0 ]]; do
+ arg="$1"
+ case "$arg" in
+ (rockspec)
+ alts+=(':rock file:{_files -g "*.rockspec"}')
+ shift 1
+ continue
+ ;;
+ (rockpack)
+ alts+=(':rock file:{_files -g "*.src.rock"}')
+ shift 1
+ continue
+ ;;
+ (external)
+ alts+=(':external rock:')
+ shift 1
+ continue
+ ;;
+ (installed)
+ tree="$2"
+ alts+=(":local rock:{__luarocks_installed_rocks ${tree}}")
+ if [[ -z "${tree}" ]]; then
+ shift
+ else
+ shift 2
+ fi
+ continue
+ ;;
+ esac
+ shift
+ continue
+ done
+ _alternative ${alts[@]}
+}
+# }}}
+# {{{ helper: git tags
+(( $+functions[__luarocks_git_tags] )) ||
+__luarocks_git_tags(){
+ autoload +X _git
+ local _git_def="$(whence -v _git)"
+ . ${_git_def##* }
+ type __git_tags &> /dev/null
+ if [[ $? != 1 ]]; then
+ __git_tags
+ fi
+}
+# }}}
+
+# {{{ `build` command
+# arguments:
+# - must: .rockspec file / external rock
+# - optional: version (only when chossing external rock)
+local make_command_options=(
+ '--pack-binary-rock[Produce a .rock file with the contents of compilation inside the current directory instead of installing it]'
+ '--keep[Do not remove previously installed versions of the rock after building a new one]'
+ '--branch=[Override the `source.branch` field in the loaded rockspec]:NAME:{_message "branch name"}'
+)
+local build_command_options=(
+ "${make_command_options[@]}"
+ '--only-deps[Installs only the dependencies of the rock]'
+ $option_deps_mode
+)
+(( $+functions[_luarocks_build] )) ||
+_luarocks_build(){
+ _arguments -A "-*" \
+ "${build_command_options[@]}" \
+ '1: :{__luarocks_rock "rockspec" "external"}' \
+ '2:: :{__luarocks_rock_version "external_or_local"}'
+}
+# }}}
+# {{{ `config` command
+# arguments:
+# - must: option
+local config_command_options=(
+ '--lua-incdir[Path to Lua header files]'
+ '--lua-libdir[Path to Lua library files]'
+ '--lua-ver[Lua version (in major.minor format)]'
+ '--system-config[Location of the system config file]'
+ '--user-config[Location of the user config file]'
+ '--rock-trees[Rocks trees in useFirst the user tree, then the system tree]'
+)
+(( $+functions[_luarocks_config] )) ||
+_luarocks_config(){
+ _arguments "${config_command_options[@]}"
+}
+# }}}
+# {{{ `doc` command
+# arguments:
+# - must: installed rock
+local doc_command_options=(
+ '--home[Open the home page of project]'
+ '--list[List documentation files only]'
+)
+(( $+functions[_luarocks_doc] )) ||
+_luarocks_doc(){
+ _arguments \
+ "${doc_command_options[@]}" \
+ '1: :{__luarocks_rock "installed" '"${opt_args[--tree]}"'}'
+}
+# }}}
+# {{{ `download` command
+# arguments:
+# - must: external only rockspec
+local download_command_options=(
+ '--all[Download all files if there are multiple matches]'
+ '--source[Download .src.rock if available]'
+ '--rockspec[Download .rockspec if available]'
+ '--arch=[Download rock for a specific architecture]:ARCH:'
+)
+(( $+functions[_luarocks_download] )) ||
+_luarocks_download(){
+ _arguments -A "-*" \
+ "${download_command_options[@]}" \
+ '1: :{__luarocks_rock "external"}' \
+ '2:: :{__luarocks_rock_version "external_or_local"}'
+}
+# }}}
+# {{{ `help` command
+# arguments:
+# must: luarocks sub command
+(( $+functions[_luarocks_help] )) ||
+_luarocks_help(){
+ _arguments '1: :__luarocks_command'
+}
+# }}}
+# {{{ `install` command
+# arguments:
+# - must: .rockspec file / external rock
+# - optional: version
+# NOTE: it receives the same argument as the build command and it accepts the same options as well
+(( $+functions[_luarocks_install] )) ||
+_luarocks_install(){
+ _luarocks_build
+}
+# }}}
+# {{{ `lint` command
+# arguments:
+# - must: rockspec file (first and last)
+(( $+functions[_luarocks_lint] )) ||
+_luarocks_lint(){
+ _arguments '1::{__luarocks_rock "rockspec"}'
+}
+# }}}
+# {{{ `list` command
+# NOTE: receives only options
+local list_command_options=(
+ '--outdated[List only rocks for which there is a higher version available in the rocks server]'
+ '--porcelain[Produce machine-friendly output]'
+)
+(( $+functions[_luarocks_list] )) ||
+_luarocks_list(){
+ _arguments "${list_command_options[@]}"
+}
+# }}}
+# {{{ `make` command
+# arguments:
+# - optional: rockspec file
+# NOTE: it's options were already described above.
+(( $+functions[_luarocks_make] )) ||
+_luarocks_make(){
+ _arguments '1:: :{__luarocks_rock "rockspec"}'
+}
+# }}}
+# {{{ `new_version` command
+# arguments:
+# - optional: .rockspec file / external rock
+# - optional: version (unless a --tag was given)
+# - optional: URL
+local new_version_command_options=(
+ '--tag=[if no version is specified, this option'"'"'s argument is used instead]:TAG:__luarocks_git_tags'
+)
+(( $+functions[_luarocks_new_version] )) ||
+_luarocks_new_version(){
+ _arguments -A "-*" \
+ "${new_version_command_options[@]}" \
+ '1:: :{__luarocks_rock "external" "rockspec"}' \
+ '2:: :{__luarocks_rock_version "external_or_local"}' \
+ '3:: :_urls'
+}
+# }}}
+# {{{ `pack` command
+# arguments:
+# - must: .rockspec file / external rock
+# - optional: version
+(( $+functions[_luarocks_pack] )) ||
+_luarocks_pack(){
+ _luarocks_build
+}
+# }}}
+# {{{ `path` command
+# NOTE: receives only options
+local path_command_options=(
+ '--bin[Adds the system path to the output]'
+ '--append[Appends the paths to the existing paths]'
+ '--lr-path[Exports the Lua path (not formatted as shell command)]'
+ '--lr-cpath[Exports the Lua cpath (not formatted as shell command)]'
+ '--lr-bin[Exports the system path (not formatted as shell command)]'
+)
+(( $+functions[_luarocks_path] )) ||
+_luarocks_path(){
+ _arguments "${path_command_options[@]}"
+}
+# }}}
+# {{{ `purge` command
+# NOTE: receives only options yet --tree is mandatory
+# NOTE: --force can be used only in conjunction with --old-versions
+local option_force='--force[Force removing old versions when]'
+local purge_command_options=(
+ '--old-versions[Keep the highest-numbered version of each rock and remove the other ones]'
+ $option_force
+)
+(( $+functions[_luarocks_purge] )) ||
+_luarocks_purge(){
+ _arguments "${purge_command_options[@]}"
+}
+# }}}
+# {{{ `remove` command
+# arguments:
+# - must: locally installed rock
+# - optional: version
+local option_force_fast='--force-fast[works like --force but doesn'"'"'t reports forced removals]'
+local remove_command_options=(
+ $option_deps_mode
+ $option_force
+ $option_force_fast
+)
+(( $+functions[_luarocks_remove] )) ||
+_luarocks_remove(){
+ _arguments -A "-*" \
+ "${remove_command_options[@]}" \
+ '1: :{__luarocks_rock "installed" '"${opt_args[--tree]}"'}' \
+ '2:: :{__luarocks_rock_version "installed" '"${opt_args[--tree]}"'}'
+}
+# }}}
+# {{{ `search` command
+# arguments:
+# - must: string as a search query
+local search_command_options=(
+ '--source[Return only rockspecs and source rocks]'
+ '--binary[Return only pure Lua and binary rocks (rocks that can be used with the "install" command without requiring a C toolchain)]'
+ '--all[List all contents of the server that are suitable to this platform, do not filter by name]'
+)
+(( $+functions[_luarocks_search] )) ||
+_luarocks_search(){
+ _arguments \
+ "${search_command_options[@]}" \
+ '*:SEARCH QUERY:'
+}
+# }}}
+# {{{ `show` command
+# arguments:
+# - must: installed rock
+local show_command_options=(
+ '--home[home page of project]'
+ '--modules[all modules provided by this package as used by require()]'
+ '--deps[packages this package depends on]'
+ '--rockspec[the full path of the rockspec file]'
+ '--mversion[the package version]'
+ '--rock-tree[local tree where rock is installed]'
+ '--rock-dir[data directory of the installed rock]'
+)
+(( $+functions[_luarocks_show] )) ||
+_luarocks_show(){
+ _arguments \
+ "${show_command_options[@]}" \
+ "1: :{__luarocks_rock 'installed' "${opt_args[--tree]}"}"
+}
+# }}}
+# {{{ `unpack` command
+# arguments:
+# - must: rockpack file / external rock
+# - optional: version (only when chossing external rock)
+local unpack_command_options=(
+ '--force[Unpack files even if the output directory already exists]'
+)
+(( $+functions[_luarocks_unpack] )) ||
+_luarocks_unpack(){
+ _arguments \
+ "${unpack_command_options[@]}" \
+ '1: :{__luarocks_rock "rockpack" "external"}'
+}
+# }}}
+# {{{ `upload` command
+# arguments:
+# - must: rockspec file
+local upload_command_options=(
+ '--skip-pack[Do not pack and send source rock]'
+ '--api-key=[Give it an API key]:KEY:{_message "api key"}'
+ '--force[Replace existing rockspec if the same revision of a module already exists]'
+)
+(( $+functions[_luarocks_upload] )) ||
+_luarocks_upload(){
+ _arguments \
+ "${upload_command_options[@]}" \
+ '1: :{__luarocks_rock "rockspec"}'
+}
+# }}}
+# {{{ `write_rockspec` command
+# arguments:
+# - optional: name
+# - optional: version
+# - optional: URL / PATH
+# receives as an argument a name and a version with optionally a URL/PATH
+local write_rockspec_command_options=(
+ '--output=[Write the rockspec with the given filename]:FILE:_files'
+ '--license=[A license string, ]:LICENSE:{_message -e "write a license string such as "MIT/X11" or "GNU GPL v3"}'
+ '--summary=[A short one-line description summary]:SUMMARY_TEXT:{_message -e "write a short summary of the rock"}'
+ '--detailed=[A longer description string]:DETAILED_TEXT:{_message -e "write a detailed description of the rock"}'
+ '--homepage=[Project homepage]:URL:_urls'
+ '--lua-version=[Supported Lua versions]:LUA_VER:__luarocks_lua_versions'
+ '--rockspec-format=[Rockspec format version, such as "1.0" or "1.1"]:VER: '
+ '--tag=[Tag to use. Will attempt to extract version number from it]:TAG:__git_tag'
+ '--lib=[A comma-separated list of libraries that C files need to link to]:'
+)
+(( $+functions[_luarocks_write_rockspec] )) ||
+_luarocks_write_rockspec(){
+ _arguments -A "-*" \
+ "${write_rockspec_command_options[@]}" \
+ '1:: :{_message "new rock name"}' \
+ '2:: :{__luarocks_rock_version "new_rock"}' \
+ '3:: :_urls'
+}
+# }}}
+
+# The real thing
+_arguments -C \
+ '(--server --only-server)--server=[Fetch rocks/rockspecs from this server]:HOST:_hosts' \
+ '(--server --only-server)--only-server=[Fetch rocks/rockspecs from this server only]:HOST:_hosts' \
+ '--only-sources=[Restrict downloads to paths matching the given URL]:URL:_urls' \
+ '--tree=[Which tree to operate on]:TREE:{_files -/}' \
+ '--local[Use the tree in the user'"'"'s home directory]' \
+ '--verbose[Display verbose output of commands executed]' \
+ '--timeout=[Timeout on network operations]:SECONDS:{_message "timeout (seconds)"}' \
+ '1: :__luarocks_command' \
+ '*::arg:->args'
+
+case "$state" in
+ (args)
+ curcontext="${curcontext%:*:*}:luarocks_${words[1]}:"
+ # check if a command with a defined completion was typed
+ type _luarocks_${words[1]} &> /dev/null
+ if [[ $? != 1 ]]; then
+ _luarocks_${words[1]}
+ fi
+esac
--
2.17.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-26 16:14 ` [PATCH 1/1] Squashed commit of the following: doron.behar
@ 2018-05-28 10:48 ` Oliver Kiddle
2018-05-29 15:38 ` Doron Behar
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Kiddle @ 2018-05-28 10:48 UTC (permalink / raw)
To: doron.behar; +Cc: zsh-workers
doron.behar@gmail.com wrote:
Thanks for the contribution!
Below are some comments on the function.
> Completion/Unix/Command/_luarocks | 560 ++++++++++++++++++++++++++++++
> 1 file changed, 560 insertions(+)
> create mode 100644 Completion/Unix/Command/_luarocks
>
> diff --git a/Completion/Unix/Command/_luarocks b/Completion/Unix/Command/_luarocks
> new file mode 100644
> index 000000000..a02bd06b5
> --- /dev/null
> +++ b/Completion/Unix/Command/_luarocks
> @@ -0,0 +1,560 @@
> +#compdef luarocks
> +
> +# {{{ helper: luarocks commands
I'd remove editor specific folding comments like {{{. Most editors can
be configured to parse the language syntax rather than requiring special
comments.
> +(( $+functions[__luarocks_command] )) ||
> +__luarocks_command(){
Normal convention would be to give helper functions like this and
__luarocks_deps_mode plural names, similar to _files being _files not
_file. So, _luarocks_commands and _luarocks_dep_modes
For subcommands, such as completing after 'luarocks unpack', the
convention would be _luarocks-unpack. So hyphens rather than
underscores.
> + build:'Build/compile a rock'
Convention is not to capitalise descriptions - 'build' rather than
'Build'.
> + help:'Help on commands'
> +local option_deps_mode='--deps-mode=[How to handle dependencies]:MODE:__luarocks_deps_mode'
It is a good idea to scan down all the descriptions and they should
nearly always start with a verb (or "don't" followed by a verb). In
these two cases, inserting "display" and "specify", respectively wil
make the descriptions clearer.
The "MODE" heading for mode matches should be in lower case.
> +# }}}
> +# {{{ helper: versions of an external rock or nothing for rockspec file
> +(( $+functions[__luarocks_rock_version] )) ||
> +__luarocks_rock_version(){
> + local i=2
> + while [[ -n "${words[$i]}" ]]; do
Should this loop perhaps stop when i reaches CURRENT? Keep in mind that
the cursor can be in the middle of a command-line and not only ever at
the end of it.
> + local user_manifest_last_date_modified="$(date -r ${HOME}/.luarocks/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
> + local system_manifest_last_date_modified="$(date -r /usr/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
date -r is not portable.
Use [[ file1 -nt file2 ]] for file newer-than style tests..
Is there a better alternative to hard-coding paths like
/usr/lib/luarocks/rocks-5.3
That is, can you query lua or luarocks for it?
At the very least I'd use a wildcard to match the version.
> +# Used to complete one or more of the followings:
> +# - .rockspec file
> +# - .src.rock file
> +# - external rock
> +(( $+functions[__luarocks_rock] )) ||
> +__luarocks_rock(){
> + local -a alts=()
> + while [[ $# -gt 0 ]]; do
> + arg="$1"
> + case "$arg" in
> + (rockspec)
> + alts+=(':rock file:{_files -g "*.rockspec"}')
What purpose are the curly brackets serving here? They just prevent
_alternative from passing any description on to _files. We normally put
(-.) in the glob to avoid picking up directories as directories are
handled differently within _files. So;
alts+=('files:rock file:_files -g "*.rockspec(-.)"')
> +(( $+functions[__luarocks_git_tags] )) ||
> +__luarocks_git_tags(){
> + autoload +X _git
> + local _git_def="$(whence -v _git)"
> + . ${_git_def##* }
> + type __git_tags &> /dev/null
> + if [[ $? != 1 ]]; then
> + __git_tags
> + fi
> +}
I'm not sure what the best approach is here but this hack doesn't look
like the best idea.
> + '--tag=[if no version is specified, this option'"'"'s argument is used instead]:TAG:__luarocks_git_tags'
To avoid the '"'"' mess, use double quotes outside for the rest of the
line. Then you can use ' for an apostrophe without any problem. As
before, TAG should be in lowercase.
> +local path_command_options=(
> + '--bin[Adds the system path to the output]'
> + '--append[Appends the paths to the existing paths]'
> + '--lr-path[Exports the Lua path (not formatted as shell command)]'
> + '--lr-cpath[Exports the Lua cpath (not formatted as shell command)]'
> + '--lr-bin[Exports the system path (not formatted as shell command)]'
It is best to stick to the shorter verb forms - "add", "append",
"export" - in the descriptions for consistency. The difference in
meaning is only subtle, perhaps attributing the action more to the user
than the command. Sometimes, this matters more as with the verb
"specify" when the option takes an argument.
> + '--license=[A license string, ]:LICENSE:{_message -e "write a license string such as "MIT/X11" or "GNU GPL v3"}'
In this spec, you already have already specified a message - "LICENSE"
so calling _message is superfluous. "LICENSE" can be improved on as a
message but I'd avoid constructs like "write a" and use something like
'license (e.g. MIT/X11 or "GNU GPL v3")'
> + '--summary=[A short one-line description summary]:SUMMARY_TEXT:{_message -e "write a short summary of the rock"}'
> + '--detailed=[A longer description string]:DETAILED_TEXT:{_message -e "write a detailed description of the rock"}'
Same again.
> + '--rockspec-format=[Rockspec format version, such as "1.0" or "1.1"]:VER: '
You can probably enumerate the possible values. In general, specific
hints on the format of an argument are more useful in the heading for
the argument than the description that is presented before the argument
has been typed. e.g:
'--rockspec-format=[specify rockspec format version]:version:(1.0 1.1)'
> + '--lib=[A comma-separated list of libraries that C files need to link to]:'
Your description, looks like the message for the matches:
'--lib[specify libraries to link against C files]:libraries (comma-separated)'
Or use _sequence or _values if you generate matches.
> + curcontext="${curcontext%:*:*}:luarocks_${words[1]}:"
Convention is a hyphen not an underscore there: luarocks-subcommand
> + # check if a command with a defined completion was typed
> + type _luarocks_${words[1]} &> /dev/null
> + if [[ $? != 1 ]]; then
> + _luarocks_${words[1]}
> + fi
This is what _call_function is there for.
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-28 10:48 ` Oliver Kiddle
@ 2018-05-29 15:38 ` Doron Behar
2018-05-29 22:00 ` Oliver Kiddle
0 siblings, 1 reply; 14+ messages in thread
From: Doron Behar @ 2018-05-29 15:38 UTC (permalink / raw)
To: zsh-workers
Thanks for your reply Oliver. I'm preparing a new and improved patch
according to your comments. Yet I wasn't sure what to do as for some of
the comments so I'm writing here my replies for your comments down
below.
On Mon, May 28, 2018 at 12:48:59PM +0200, Oliver Kiddle wrote:
> doron.behar@gmail.com wrote:
>
> Thanks for the contribution!
>
> Below are some comments on the function.
>
> > Completion/Unix/Command/_luarocks | 560 ++++++++++++++++++++++++++++++
> > 1 file changed, 560 insertions(+)
> > create mode 100644 Completion/Unix/Command/_luarocks
> >
> > diff --git a/Completion/Unix/Command/_luarocks b/Completion/Unix/Command/_luarocks
> > new file mode 100644
> > index 000000000..a02bd06b5
> > --- /dev/null
> > +++ b/Completion/Unix/Command/_luarocks
> > @@ -0,0 +1,560 @@
> > +#compdef luarocks
> > +
> > +# {{{ helper: luarocks commands
>
> I'd remove editor specific folding comments like {{{. Most editors can
> be configured to parse the language syntax rather than requiring special
> comments.
>
I find that extremely useful when I write completions since it's super
comfortable with my editor (Vim). Do you have any suggestions? Perhaps
using EditorConfig?
> > +(( $+functions[__luarocks_command] )) ||
> > +__luarocks_command(){
>
> Normal convention would be to give helper functions like this and
> __luarocks_deps_mode plural names, similar to _files being _files not
> _file. So, _luarocks_commands and _luarocks_dep_modes
>
Done.
> For subcommands, such as completing after 'luarocks unpack', the
> convention would be _luarocks-unpack. So hyphens rather than
> underscores.
>
> > + build:'Build/compile a rock'
>
> Convention is not to capitalise descriptions - 'build' rather than
> 'Build'.
>
Done.
> > + help:'Help on commands'
> > +local option_deps_mode='--deps-mode=[How to handle dependencies]:MODE:__luarocks_deps_mode'
>
> It is a good idea to scan down all the descriptions and they should
> nearly always start with a verb (or "don't" followed by a verb). In
> these two cases, inserting "display" and "specify", respectively wil
> make the descriptions clearer.
>
Done, used this:
local option_deps_modes='--deps-mode=[specify how to handle dependencies]:mode:__luarocks_deps_modes'
> The "MODE" heading for mode matches should be in lower case.
>
Done.
> > +# }}}
> > +# {{{ helper: versions of an external rock or nothing for rockspec file
> > +(( $+functions[__luarocks_rock_version] )) ||
> > +__luarocks_rock_version(){
> > + local i=2
> > + while [[ -n "${words[$i]}" ]]; do
>
> Should this loop perhaps stop when i reaches CURRENT? Keep in mind that
> the cursor can be in the middle of a command-line and not only ever at
> the end of it.
>
> > + local user_manifest_last_date_modified="$(date -r ${HOME}/.luarocks/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
> > + local system_manifest_last_date_modified="$(date -r /usr/lib/luarocks/rocks-5.3/manifest +%s 2>/dev/null)"
>
> date -r is not portable.
>
> Use [[ file1 -nt file2 ]] for file newer-than style tests..
Done.
>
> Is there a better alternative to hard-coding paths like
> /usr/lib/luarocks/rocks-5.3
>
> That is, can you query lua or luarocks for it?
There is, It's `luarocks config --lua-ver`. I'll use it.
> At the very least I'd use a wildcard to match the version.
>
> > +# Used to complete one or more of the followings:
> > +# - .rockspec file
> > +# - .src.rock file
> > +# - external rock
> > +(( $+functions[__luarocks_rock] )) ||
> > +__luarocks_rock(){
> > + local -a alts=()
> > + while [[ $# -gt 0 ]]; do
> > + arg="$1"
> > + case "$arg" in
> > + (rockspec)
> > + alts+=(':rock file:{_files -g "*.rockspec"}')
>
> What purpose are the curly brackets serving here? They just prevent
> _alternative from passing any description on to _files. We normally put
> (-.) in the glob to avoid picking up directories as directories are
> handled differently within _files. So;
> alts+=('files:rock file:_files -g "*.rockspec(-.)"')
Besides removing the curly brackets, what is the difference when I use
'(-.)' in the glob and when I don't? I tried to test this in directories
where I had files ending with `.rockspec` and in directories where I
hadn't had files like these and I couldn't tell the difference in
behaviour. What am I missing?
>
> > +(( $+functions[__luarocks_git_tags] )) ||
> > +__luarocks_git_tags(){
> > + autoload +X _git
> > + local _git_def="$(whence -v _git)"
> > + . ${_git_def##* }
> > + type __git_tags &> /dev/null
> > + if [[ $? != 1 ]]; then
> > + __git_tags
> > + fi
> > +}
>
> I'm not sure what the best approach is here but this hack doesn't look
> like the best idea.
I couldn't think of a better way of doing this other then just copying
manually `__git_tags` or using this hack.
>
> > + '--tag=[if no version is specified, this option'"'"'s argument is used instead]:TAG:__luarocks_git_tags'
>
> To avoid the '"'"' mess, use double quotes outside for the rest of the
> line. Then you can use ' for an apostrophe without any problem.
Done. Yea you're right. I guess I was lazy to modify the whole quotes.
> As before, TAG should be in lowercase.
Done. Oh right, I got it. `TAG`s in lower case.. I guess I was influenced by
unprofessional completion functions from
https://github.com/zsh-users/zsh-completions.
>
> > +local path_command_options=(
> > + '--bin[Adds the system path to the output]'
> > + '--append[Appends the paths to the existing paths]'
> > + '--lr-path[Exports the Lua path (not formatted as shell command)]'
> > + '--lr-cpath[Exports the Lua cpath (not formatted as shell command)]'
> > + '--lr-bin[Exports the system path (not formatted as shell command)]'
>
> It is best to stick to the shorter verb forms - "add", "append",
> "export" - in the descriptions for consistency. The difference in
> meaning is only subtle, perhaps attributing the action more to the user
> than the command. Sometimes, this matters more as with the verb
> "specify" when the option takes an argument.
>
> > + '--license=[A license string, ]:LICENSE:{_message -e "write a license string such as "MIT/X11" or "GNU GPL v3"}'
>
> In this spec, you already have already specified a message - "LICENSE"
> so calling _message is superfluous. "LICENSE" can be improved on as a
> message but I'd avoid constructs like "write a" and use something like
> 'license (e.g. MIT/X11 or "GNU GPL v3")'
That's a good description, cool. I've come up with this options
specifications:
'--license=[a license string]:license:{_message -e license "(e.g. \"MIT/X11\" or \"GNU GPL v3\")"}'
>
> > + '--summary=[A short one-line description summary]:SUMMARY_TEXT:{_message -e "write a short summary of the rock"}'
> > + '--detailed=[A longer description string]:DETAILED_TEXT:{_message -e "write a detailed description of the rock"}'
>
> Same again.
'--summary=[a short one-line description summary]:summary:{_message -e "short summary of the rock"}'
'--detailed=[a longer description string]:detailed_text:{_message -e "detailed description of the rock"}'
>
> > + '--rockspec-format=[Rockspec format version, such as "1.0" or "1.1"]:VER: '
>
> You can probably enumerate the possible values. In general, specific
> hints on the format of an argument are more useful in the heading for
> the argument than the description that is presented before the argument
> has been typed. e.g:
>
> '--rockspec-format=[specify rockspec format version]:version:(1.0 1.1)'
I wrote that initially because I wasn't sure according to luarocks
documentation as for this option's argument. I think I'll go with your
suggestion and if anyone will complain about 'Unsupported versions for
rockspec formats in _luarocks` We'll just add another option there.
>
> > + '--lib=[A comma-separated list of libraries that C files need to link to]:'
>
> Your description, looks like the message for the matches:
> '--lib[specify libraries to link against C files]:libraries (comma-separated)'
>
> Or use _sequence or _values if you generate matches.
>
I've no idea if this is possible to complete matches for this option. I
don't have any experience in using this option as well so I'll leave it
this way. Perhaps it'll be better for the specification to be this:
'--lib=[C library files to link to]:'
?
> > + curcontext="${curcontext%:*:*}:luarocks_${words[1]}:"
>
> Convention is a hyphen not an underscore there: luarocks-subcommand
>
Done.
> > + # check if a command with a defined completion was typed
> > + type _luarocks_${words[1]} &> /dev/null
> > + if [[ $? != 1 ]]; then
> > + _luarocks_${words[1]}
> > + fi
>
> This is what _call_function is there for.
Done. Good call, I'll use that in the next completions I'll write as
well.
I'm not sure I understand yet what tags are used for generally in
completions (RTFD I know but forgive me I'm lazy :/) but anyway the tag
I chose as the 1st argument for `_call_function` is `subcmd`. Is that
cool? It looks like that:
_call_function subcmd _luarocks_${words[1]}
>
> Oliver
Doron
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-29 15:38 ` Doron Behar
@ 2018-05-29 22:00 ` Oliver Kiddle
2018-05-30 12:47 ` Doron Behar
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Kiddle @ 2018-05-29 22:00 UTC (permalink / raw)
To: zsh-workers
Doron Behar wrote:
>
> I find that extremely useful when I write completions since it's super
> comfortable with my editor (Vim). Do you have any suggestions? Perhaps
> using EditorConfig?
I don't think editorconfig supports much beyond basic definitions of
indent style. In the case of vim, it supports a variety of foldmethods.
'set foldmethod=syntax' doesn't do such a bad job and if you don't like
it you can define custom expressions.
> Done, used this:
>
> local option_deps_modes='--deps-mode=[specify how to handle dependencies]:mode:__luarocks_deps_modes'
That looks fine.
> Besides removing the curly brackets, what is the difference when I use
> '(-.)' in the glob and when I don't? I tried to test this in directories
> where I had files ending with `.rockspec` and in directories where I
> hadn't had files like these and I couldn't tell the difference in
> behaviour. What am I missing?
It can make a difference if you have directories ending in .rockspec.
That perhaps isn't common but in some contexts it can matter.
It is common to setup the file-patterns style to mix directories in with
matched files in which case it is less apparent.
For reference, see the old discussion around workers/19292.
> I couldn't think of a better way of doing this other then just copying
> manually `__git_tags` or using this hack.
Duplicating __git_tags' functionality is just as few lines of code and
less likely to break. This looks rather fragile.
> > In this spec, you already have already specified a message - "LICENSE"
> > so calling _message is superfluous. "LICENSE" can be improved on as a
> > message but I'd avoid constructs like "write a" and use something like
> > 'license (e.g. MIT/X11 or "GNU GPL v3")'
>
> That's a good description, cool. I've come up with this options
> specifications:
>
> '--license=[a license string]:license:{_message -e license "(e.g. \"MIT/X11\" or \"GNU GPL v3\")"}'
My point about calling _message being superfluous perhaps wasn't clear.
The second part of the colon-delimited spec IS a message. So you can
just use:
'--license=[specify a license string]:license (e.g. "MIT/X11" or "GNU GPL v3")'
Note that I also added an initial verb in the description which was a
suggestion I had made earlier but not repeated.
> > Your description, looks like the message for the matches:
> > '--lib[specify libraries to link against C files]:libraries (comma-separated)'
> >
> > Or use _sequence or _values if you generate matches.
> >
>
> I've no idea if this is possible to complete matches for this option. I
> don't have any experience in using this option as well so I'll leave it
> this way. Perhaps it'll be better for the specification to be this:
>
> '--lib=[C library files to link to]:'
At least include a message after the colon.
> I'm not sure I understand yet what tags are used for generally in
> completions (RTFD I know but forgive me I'm lazy :/) but anyway the tag
> I chose as the 1st argument for `_call_function` is `subcmd`. Is that
> cool? It looks like that:
>
> _call_function subcmd _luarocks_${words[1]}
The first argument to _call_function is not a tag but a variable name.
The return status from the function is assigned to it. Normally, you'll
see "ret".
Tags are used with the zstyle mechanism. zstyle gives you
context-sensitive options where a bunch of information is stuffed into a
context string. For completion,
:completion:<function>:<completer>:<command>:<arg>:<tag>
Note that the last component is the tag for the current matches.
This is a much more succinct and convenient way to configure things than
if you needed lots of nested if statements.
Taking an example from one of your messages we have:
zstyle ':completion:*:kill:*' command 'ps -u $USER -o pid,%cpu,tty,cputime,cmd'
As it happens, for kill completion the command style is only looked up
when completing processes but it'd be wise to put the processes tag
into the context there. If you want the style to work for all commands
and not just kill, that'd be essential. Otherwise, it might run ps when
generating some other entirely different sort of matches:
zstyle ':completion:*:processes' command 'ps -u $USER -o pid,%cpu,tty,cputime,cmd'
(To answer the question in that other post, don't try to emulate that,
just use _pids and leave process selection to the user. ps defaulting to
the current tty made more sense a couple of decades ago.)
The other thing tags are often used for is for grouping completion
matches. Groups are separate but it is common to use the tag as the
group name which is what you get with:
zstyle ':completion:*' group-name ''
Finally, tags can be used with the tag-order style to allow users to
try some matches before others.
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-29 22:00 ` Oliver Kiddle
@ 2018-05-30 12:47 ` Doron Behar
2018-05-30 15:43 ` Oliver Kiddle
0 siblings, 1 reply; 14+ messages in thread
From: Doron Behar @ 2018-05-30 12:47 UTC (permalink / raw)
To: zsh-workers
Thank you Oliver for your guidance, The revised patch is ready I'll send
it shortly, here are some more questions / comments I had regarding your
last reply.
On Wed, May 30, 2018 at 12:00:49AM +0200, Oliver Kiddle wrote:
> Doron Behar wrote:
> >
> > I find that extremely useful when I write completions since it's super
> > comfortable with my editor (Vim). Do you have any suggestions? Perhaps
> > using EditorConfig?
>
> I don't think editorconfig supports much beyond basic definitions of
> indent style. In the case of vim, it supports a variety of foldmethods.
> 'set foldmethod=syntax' doesn't do such a bad job and if you don't like
> it you can define custom expressions.
It's great, thanks!
>
> > Done, used this:
> >
> > local option_deps_modes='--deps-mode=[specify how to handle dependencies]:mode:__luarocks_deps_modes'
>
> That looks fine.
>
> > Besides removing the curly brackets, what is the difference when I use
> > '(-.)' in the glob and when I don't? I tried to test this in directories
> > where I had files ending with `.rockspec` and in directories where I
> > hadn't had files like these and I couldn't tell the difference in
> > behaviour. What am I missing?
>
> It can make a difference if you have directories ending in .rockspec.
> That perhaps isn't common but in some contexts it can matter.
> It is common to setup the file-patterns style to mix directories in with
> matched files in which case it is less apparent.
> For reference, see the old discussion around workers/19292.
Got it.
>
> > I couldn't think of a better way of doing this other then just copying
> > manually `__git_tags` or using this hack.
>
> Duplicating __git_tags' functionality is just as few lines of code and
> less likely to break. This looks rather fragile.
I copied `__git_tags`s content straight from `_git`s definition and
added a comment stating so.
>
> > > In this spec, you already have already specified a message - "LICENSE"
> > > so calling _message is superfluous. "LICENSE" can be improved on as a
> > > message but I'd avoid constructs like "write a" and use something like
> > > 'license (e.g. MIT/X11 or "GNU GPL v3")'
> >
> > That's a good description, cool. I've come up with this options
> > specifications:
> >
> > '--license=[a license string]:license:{_message -e license "(e.g. \"MIT/X11\" or \"GNU GPL v3\")"}'
>
> My point about calling _message being superfluous perhaps wasn't clear.
> The second part of the colon-delimited spec IS a message. So you can
> just use:
>
> '--license=[specify a license string]:license (e.g. "MIT/X11" or "GNU GPL v3")'
>
> Note that I also added an initial verb in the description which was a
> suggestion I had made earlier but not repeated.
>
Got it.
> > > Your description, looks like the message for the matches:
> > > '--lib[specify libraries to link against C files]:libraries (comma-separated)'
> > >
> > > Or use _sequence or _values if you generate matches.
> > >
> >
> > I've no idea if this is possible to complete matches for this option. I
> > don't have any experience in using this option as well so I'll leave it
> > this way. Perhaps it'll be better for the specification to be this:
> >
> > '--lib=[C library files to link to]:'
>
> At least include a message after the colon.
This is what I chose:
'--lib=[comma separated list of C library files to link to]:library files'
>
> > I'm not sure I understand yet what tags are used for generally in
> > completions (RTFD I know but forgive me I'm lazy :/) but anyway the tag
> > I chose as the 1st argument for `_call_function` is `subcmd`. Is that
> > cool? It looks like that:
> >
> > _call_function subcmd _luarocks_${words[1]}
>
> The first argument to _call_function is not a tag but a variable name.
> The return status from the function is assigned to it. Normally, you'll
> see "ret".
O.K, I used `ret` instead yet currently I don't see any use in this
variable since I didn't structured the completion file with `return ret`
at the end of any of the functions as I've seen in other completion
functions I read.
>
> Tags are used with the zstyle mechanism. zstyle gives you
> context-sensitive options where a bunch of information is stuffed into a
> context string. For completion,
> :completion:<function>:<completer>:<command>:<arg>:<tag>
> Note that the last component is the tag for the current matches.
> This is a much more succinct and convenient way to configure things than
> if you needed lots of nested if statements.
>
> Taking an example from one of your messages we have:
> zstyle ':completion:*:kill:*' command 'ps -u $USER -o pid,%cpu,tty,cputime,cmd'
>
> As it happens, for kill completion the command style is only looked up
> when completing processes but it'd be wise to put the processes tag
> into the context there. If you want the style to work for all commands
> and not just kill, that'd be essential. Otherwise, it might run ps when
> generating some other entirely different sort of matches:
>
> zstyle ':completion:*:processes' command 'ps -u $USER -o pid,%cpu,tty,cputime,cmd'
>
> (To answer the question in that other post, don't try to emulate that,
> just use _pids and leave process selection to the user. ps defaulting to
> the current tty made more sense a couple of decades ago.)
>
> The other thing tags are often used for is for grouping completion
> matches. Groups are separate but it is common to use the tag as the
> group name which is what you get with:
> zstyle ':completion:*' group-name ''
>
> Finally, tags can be used with the tag-order style to allow users to
> try some matches before others.
>
> Oliver
Jesus, using `zstyle` is complicated, I hope I'll master this skill one
day.. Where can I find in the documentation more explanations about the
relation between zstyle and completion functions?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-30 12:47 ` Doron Behar
@ 2018-05-30 15:43 ` Oliver Kiddle
2018-06-01 7:18 ` Doron Behar
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Kiddle @ 2018-05-30 15:43 UTC (permalink / raw)
To: zsh-workers
Doron Behar wrote:
> O.K, I used `ret` instead yet currently I don't see any use in this
> variable since I didn't structured the completion file with `return ret`
> at the end of any of the functions as I've seen in other completion
> functions I read.
You need something like it in the final function. You're calling
_arguments and ignoring the return status from it. This can break things
- approximate completion mostly.
It's only really that function where you need it.
> Jesus, using `zstyle` is complicated, I hope I'll master this skill one
> day.. Where can I find in the documentation more explanations about the
> relation between zstyle and completion functions?
It looks worse than it is. You can see the styles and contexts for a
particular completion by pressing Ctrl-X h instead of Tab. With a
numeric argument (Escape 1 for emacs mode) it provides more detail.
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-05-30 15:43 ` Oliver Kiddle
@ 2018-06-01 7:18 ` Doron Behar
2018-06-01 13:30 ` Doron Behar
0 siblings, 1 reply; 14+ messages in thread
From: Doron Behar @ 2018-06-01 7:18 UTC (permalink / raw)
To: zsh-workers
Thanks again for you reply Oliver, I'll get to the issue of `ret` later
since I might need your advice on the matter of installed rocks' cache
policy function:
You asked me in one of the previous mails whether it is possible or not
to query luarocks instead of hard-coding the paths of the manifest
files. To your reminder, I compared their last modified date (Thanks to
you with `-nt` rather then `date -r`) with that of the cache files, here
is the original version after changing to `-nt`:
(( $+functions[___luarocks_installed_rocks_cache_policy] )) ||
___luarocks_installed_rocks_cache_policy(){ local cache_file="$1"
# TODO: use luarocks config to get these paths according to luarocks
local user_manifest_file="${HOME}/.luarocks/lib/luarocks/rocks-5.3/manifest"
local system_manifest_file="/usr/lib/luarocks/rocks-5.3/manifest"
if [[ -f ${user_manifest_file} ]] || [[ -f ${system_manifest_file} ]]; then
if [[ -f ${cache_file} ]]; then
# if either one of the manifest files is newer then the cache:
if [ ${user_manifest_file} -nt ${cache_file} ] || [ ${system_manifest_file} -nt ${cache_file} ]; then
(( 1 ))
else
(( 0 ))
fi
else
(( 1 ))
fi
else
(( 1 ))
fi
}
The good news as for the TODO comment I wrote there is that it is
possible to do so: Using `luarocks config --lua-ver` for the version
(now hard-coded to 5.3) and `luarocks config --rock-trees`.
However, the bad news is that I'm afraid that calling `luarocks config`
twice like that whenever I query the cache validity, is a huge
performance hit.
The solution which will most likely best solve this issue is to use a
similar cache mechanism for these values as well. This *inner* cache's
validity should be checked against the cache files' and the luarocks
configuration files (system and user) modification date. What makes this
even more complicated is the fact that the locations of the user and
system configuration file can be queried for that as well. And yet
again, even running `luarocks` twice for getting these values will bring
a performance hit as well.
Anyway, I've come to the conclusion that I'll need to write inside this
cache policy function what's written in the cache related functions
(mostly `_store_cache` and `_retrieve_cache`) and customise it for my
parameters, it'll take some time so I'll send it here when it is ready.
I think it's better to take care of `ret` afterwards.
On Wed, May 30, 2018 at 05:43:07PM +0200, Oliver Kiddle wrote:
> Doron Behar wrote:
> > O.K, I used `ret` instead yet currently I don't see any use in this
> > variable since I didn't structured the completion file with `return ret`
> > at the end of any of the functions as I've seen in other completion
> > functions I read.
>
> You need something like it in the final function. You're calling
> _arguments and ignoring the return status from it. This can break things
> - approximate completion mostly.
>
> It's only really that function where you need it.
>
> > Jesus, using `zstyle` is complicated, I hope I'll master this skill one
> > day.. Where can I find in the documentation more explanations about the
> > relation between zstyle and completion functions?
>
> It looks worse than it is. You can see the styles and contexts for a
> particular completion by pressing Ctrl-X h instead of Tab. With a
> numeric argument (Escape 1 for emacs mode) it provides more detail.
>
> Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-01 7:18 ` Doron Behar
@ 2018-06-01 13:30 ` Doron Behar
2018-06-01 22:45 ` Oliver Kiddle
0 siblings, 1 reply; 14+ messages in thread
From: Doron Behar @ 2018-06-01 13:30 UTC (permalink / raw)
To: zsh-workers
Continuing from my last reply, here is the final version of my cache
policy function. I've defined two functions that manually store in the
cache variables the cache policy function needs to know in order to
return the correct value.
I'd be glad to get some feedback, thanks!
(( $+functions[___luarocks_manually_store_cache_configs_paths] )) ||
___luarocks_manually_store_cache_configs_paths(){
user_config_path="$(_call_program user_config_path luarocks config --user-config)"
system_config_path="$(_call_program system_config_path luarocks config --system-config)"
print user_config_path=$user_config_path > ${cache_dir}/luarocks_configs_paths
print system_config_path=$system_config_path >> ${cache_dir}/luarocks_configs_paths
}
(( $+functions[___luarocks_manually_store_cache_configured_values] )) ||
___luarocks_manually_store_cache_configured_values(){
local default_trees=($(_call_program rock_trees luarocks config --rock-trees))
# The following command usually gives somethins like this
#
# /home/me/.luarocks user
# /usr system
#
# We'll just use the 1st and 3rd elements in the array for the default trees
configured_user_tree="${default_trees[1]}"
configured_system_tree="${default_trees[3]}"
configured_lua_version="$(_call_program lua_ver luarocks config --lua-ver)"
print configured_lua_version=$configured_lua_version > ${cache_dir}/luarocks_configured_values
print configured_user_tree=$configured_user_tree >> ${cache_dir}/luarocks_configured_values
print configured_system_tree=$configured_system_tree >> ${cache_dir}/luarocks_configured_values
}
(( $+functions[___luarocks_installed_rocks_cache_policy] )) ||
___luarocks_installed_rocks_cache_policy(){
local cache_file="$1"
# Before checking the modification date of the manifests files vs the
# installed rocks cache files, we need to perform the following checks:
# - luarocks executable modification date vs modification date of cache file
# holding the default configuration files' locations
# ) if configuration files' locations were possibly changed, we need to:
# * set and cache the *possibly* new locations of the configuration files
# ) else:
# * retrieve from cache the configuration files' locations
# ) end if
# - configuration files' modification date vs modification date of cache file
# holding the values from `luarocks config --lua-ver` and `luarocks config
# --rock-trees`
# ) if the configuration files' locations were changed:
# * set and cache the values from the commands above
# ) else:
# ) if configuration files are newer:
# * set and cache the values from the commands above
# ) else:
# * retrive from cache the values of the commands above
# ) end if
# ) end if
# Decide which directory to retrieve cache from, and ensure it exists
local cache_dir
zstyle -s ":completion:${curcontext}:" cache-path cache_dir
: ${cache_dir:=${ZDOTDIR:-$HOME}/.zcompcache}
if [[ ! -d "$cache_dir" ]]; then
[[ -e "$cache_dir" ]] &&
_message "cache-dir ($cache_dir) isn't a directory\!"
fi
local where_luarocks=$(where luarocks)
# luarocks_configured_values
local configured_lua_version configured_user_tree configured_system_tree
# luarocks_configs_paths
local config
if [[ -e ${cache_dir}/luarocks_configs_paths ]]; then
if [ ${where_luarocks} -nt ${cache_dir}/luarocks_configs_paths ]; then
___luarocks_manually_store_cache_configs_paths
else
. ${cache_dir}/luarocks_configs_paths
fi
else
___luarocks_manually_store_cache_configs_paths
fi
if [[ -e ${cache_dir}/luarocks_configured_values ]]; then
if [ ${user_config_path} -nt ${cache_dir}/luarocks_configured_values ] || [ ${system_config_path} -nt ${cache_dir}/luarocks_configured_values ]; then
___luarocks_manually_store_cache_configured_values
else
. ${cache_dir}/luarocks_configured_values
fi
else
___luarocks_manually_store_cache_configured_values
fi
local user_manifest_file="${configured_user_tree}/lib/luarocks/rocks-${configured_lua_version}/manifest"
local system_manifest_file="${configured_system_tree}/lib/luarocks/rocks-${configured_lua_version}/manifest"
if [[ -f ${user_manifest_file} ]] || [[ -f ${system_manifest_file} ]]; then
if [[ -f ${cache_file} ]]; then
# if either one of the manifest files is newer then the cache:
if [ ${user_manifest_file} -nt ${cache_file} ] || [ ${system_manifest_file} -nt ${cache_file} ]; then
(( 1 ))
else
(( 0 ))
fi
else
(( 1 ))
fi
else
(( 1 ))
fi
}
On Fri, Jun 01, 2018 at 10:18:54AM +0300, Doron Behar wrote:
> Thanks again for you reply Oliver, I'll get to the issue of `ret` later
> since I might need your advice on the matter of installed rocks' cache
> policy function:
>
> You asked me in one of the previous mails whether it is possible or not
> to query luarocks instead of hard-coding the paths of the manifest
> files. To your reminder, I compared their last modified date (Thanks to
> you with `-nt` rather then `date -r`) with that of the cache files, here
> is the original version after changing to `-nt`:
>
> (( $+functions[___luarocks_installed_rocks_cache_policy] )) ||
> ___luarocks_installed_rocks_cache_policy(){ local cache_file="$1"
> # TODO: use luarocks config to get these paths according to luarocks
> local user_manifest_file="${HOME}/.luarocks/lib/luarocks/rocks-5.3/manifest"
> local system_manifest_file="/usr/lib/luarocks/rocks-5.3/manifest"
> if [[ -f ${user_manifest_file} ]] || [[ -f ${system_manifest_file} ]]; then
> if [[ -f ${cache_file} ]]; then
> # if either one of the manifest files is newer then the cache:
> if [ ${user_manifest_file} -nt ${cache_file} ] || [ ${system_manifest_file} -nt ${cache_file} ]; then
> (( 1 ))
> else
> (( 0 ))
> fi
> else
> (( 1 ))
> fi
> else
> (( 1 ))
> fi
> }
>
> The good news as for the TODO comment I wrote there is that it is
> possible to do so: Using `luarocks config --lua-ver` for the version
> (now hard-coded to 5.3) and `luarocks config --rock-trees`.
>
> However, the bad news is that I'm afraid that calling `luarocks config`
> twice like that whenever I query the cache validity, is a huge
> performance hit.
>
> The solution which will most likely best solve this issue is to use a
> similar cache mechanism for these values as well. This *inner* cache's
> validity should be checked against the cache files' and the luarocks
> configuration files (system and user) modification date. What makes this
> even more complicated is the fact that the locations of the user and
> system configuration file can be queried for that as well. And yet
> again, even running `luarocks` twice for getting these values will bring
> a performance hit as well.
>
> Anyway, I've come to the conclusion that I'll need to write inside this
> cache policy function what's written in the cache related functions
> (mostly `_store_cache` and `_retrieve_cache`) and customise it for my
> parameters, it'll take some time so I'll send it here when it is ready.
> I think it's better to take care of `ret` afterwards.
>
> On Wed, May 30, 2018 at 05:43:07PM +0200, Oliver Kiddle wrote:
> > Doron Behar wrote:
> > > O.K, I used `ret` instead yet currently I don't see any use in this
> > > variable since I didn't structured the completion file with `return ret`
> > > at the end of any of the functions as I've seen in other completion
> > > functions I read.
> >
> > You need something like it in the final function. You're calling
> > _arguments and ignoring the return status from it. This can break things
> > - approximate completion mostly.
> >
> > It's only really that function where you need it.
> >
> > > Jesus, using `zstyle` is complicated, I hope I'll master this skill one
> > > day.. Where can I find in the documentation more explanations about the
> > > relation between zstyle and completion functions?
> >
> > It looks worse than it is. You can see the styles and contexts for a
> > particular completion by pressing Ctrl-X h instead of Tab. With a
> > numeric argument (Escape 1 for emacs mode) it provides more detail.
> >
> > Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-01 13:30 ` Doron Behar
@ 2018-06-01 22:45 ` Oliver Kiddle
2018-06-03 21:46 ` Daniel Shahaf
2018-06-05 15:41 ` Doron Behar
0 siblings, 2 replies; 14+ messages in thread
From: Oliver Kiddle @ 2018-06-01 22:45 UTC (permalink / raw)
To: zsh-workers
Doron Behar wrote:
> However, the bad news is that I'm afraid that calling `luarocks config`
> twice like that whenever I query the cache validity, is a huge
> performance hit.
> The solution which will most likely best solve this issue is to use a
> similar cache mechanism for these values as well. This *inner* cache's
Sounds rather complicated but perhaps necessary depending on just how
slow it is. A global variable – typically named _cache_… – is one
option for caching that avoids much of the complexity of the disk cache
mechanism if the lifetime of the session is an appropriate policy for
how long to retain the cached information.
> I'd be glad to get some feedback, thanks!
> (( $+functions[___luarocks_manually_store_cache_configs_paths] )) ||
> ___luarocks_manually_store_cache_configs_paths(){
> user_config_path="$(_call_program user_config_path luarocks config --user-config)"
> system_config_path="$(_call_program system_config_path luarocks config --system-config)"
These variables should be declared local. If the intention is for them
to be global, use typeset -g and prefix the names with something like
_cache_luarocks_.
> print user_config_path=$user_config_path > ${cache_dir}/luarocks_configs_paths
> print system_config_path=$system_config_path >> ${cache_dir}/luarocks_configs_paths
You might need to quote the values with ${(qq)user_config_path} in case
they have spaces in their values. By using braces around the print
statements only one redirection will be needed instead of the
redirection and append: { print ...; print ... } > cache
> local where_luarocks=$(where luarocks)
Use $commands[luarocks] rather than where in a command substitution.
Command substitution typically requires a forked subshell which will be
less efficient.
> # luarocks_configured_values
> local configured_lua_version configured_user_tree configured_system_tree
> # luarocks_configs_paths
> local config
> if [[ -e ${cache_dir}/luarocks_configs_paths ]]; then
> if [ ${where_luarocks} -nt ${cache_dir}/luarocks_configs_paths ]; then
It is generally better to use [[ ... ]] for all conditions unless you're
writing a script targeted at /bin/sh – which is not the case here.
> if [[ -f ${user_manifest_file} ]] || [[ -f ${system_manifest_file} ]]; then
> if [[ -f ${cache_file} ]]; then
> # if either one of the manifest files is newer then the cache:
> if [ ${user_manifest_file} -nt ${cache_file} ] || [ ${system_manifest_file} -nt ${cache_file} ]; then
> (( 1 ))
> else
> (( 0 ))
> fi
> else
> (( 1 ))
> fi
> else
> (( 1 ))
> fi
I find this (( 1 )) stuff confusing. If I'm not mistaken the whole thing
is equivalent to:
[[ ( ! -f ${user_manifest_file} && ! -f ${system_manifest_file} ) ||
! -f ${cache_file} || ${user_manifest_file} -nt ${cache_file} ||
${system_manifest_file} -nt ${cache_file} ]]
As for the logic, I'd mainly question what happens when one but not both
of the manifest files is found to not exist.
Note that && and || can be used inside [[ ... ]].
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-01 22:45 ` Oliver Kiddle
@ 2018-06-03 21:46 ` Daniel Shahaf
2018-06-05 15:41 ` Doron Behar
1 sibling, 0 replies; 14+ messages in thread
From: Daniel Shahaf @ 2018-06-03 21:46 UTC (permalink / raw)
To: zsh-workers
Oliver Kiddle wrote on Sat, Jun 02, 2018 at 00:45:17 +0200:
> Doron Behar wrote:
> > print user_config_path=$user_config_path > ${cache_dir}/luarocks_configs_paths
> > print system_config_path=$system_config_path >> ${cache_dir}/luarocks_configs_paths
>
> You might need to quote the values with ${(qq)user_config_path} in case
> they have spaces in their values.
… and if they can have backslashes, pass -r to print so they round-trip correctly.
(Or just use 'typeset -p')
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-01 22:45 ` Oliver Kiddle
2018-06-03 21:46 ` Daniel Shahaf
@ 2018-06-05 15:41 ` Doron Behar
2018-06-07 15:59 ` Oliver Kiddle
1 sibling, 1 reply; 14+ messages in thread
From: Doron Behar @ 2018-06-05 15:41 UTC (permalink / raw)
To: zsh-workers
On Sat, Jun 02, 2018 at 12:45:17AM +0200, Oliver Kiddle wrote:
> Doron Behar wrote:
> > However, the bad news is that I'm afraid that calling `luarocks config`
> > twice like that whenever I query the cache validity, is a huge
> > performance hit.
>
> > The solution which will most likely best solve this issue is to use a
> > similar cache mechanism for these values as well. This *inner* cache's
>
> Sounds rather complicated but perhaps necessary depending on just how
> slow it is. A global variable – typically named _cache_… – is one
> option for caching that avoids much of the complexity of the disk cache
> mechanism if the lifetime of the session is an appropriate policy for
> how long to retain the cached information.
>
> > I'd be glad to get some feedback, thanks!
>
>
> > (( $+functions[___luarocks_manually_store_cache_configs_paths] )) ||
> > ___luarocks_manually_store_cache_configs_paths(){
> > user_config_path="$(_call_program user_config_path luarocks config --user-config)"
> > system_config_path="$(_call_program system_config_path luarocks config --system-config)"
>
> These variables should be declared local. If the intention is for them
> to be global, use typeset -g and prefix the names with something like
> _cache_luarocks_.
>
> > print user_config_path=$user_config_path > ${cache_dir}/luarocks_configs_paths
> > print system_config_path=$system_config_path >> ${cache_dir}/luarocks_configs_paths
Yea I forgot to make them local alongside configured_lua_version
configured_user_tree configured_system_tree. Done, thanks.
>
> You might need to quote the values with ${(qq)user_config_path} in case
> they have spaces in their values. By using braces around the print
> statements only one redirection will be needed instead of the
> redirection and append: { print ...; print ... } > cache
You are right, the `_store_cache` function's definition, uses the
following for every non-array argument given to it:
print -r "$var=${(Pqq)^^var}"
Since my $var is known, I read the documentation and I understand what
the `P`, and double `q` mean so I'll put:
print -r var=${(qq)var}
As for the loop running running for every argument `_store_cache` gets,
it ends with >! which should be used here as well according to the
documentation..
>
> > local where_luarocks=$(where luarocks)
>
> Use $commands[luarocks] rather than where in a command substitution.
> Command substitution typically requires a forked subshell which will be
> less efficient.
Done.
>
> > # luarocks_configured_values
> > local configured_lua_version configured_user_tree configured_system_tree
> > # luarocks_configs_paths
> > local config
> > if [[ -e ${cache_dir}/luarocks_configs_paths ]]; then
> > if [ ${where_luarocks} -nt ${cache_dir}/luarocks_configs_paths ]; then
>
> It is generally better to use [[ ... ]] for all conditions unless you're
> writing a script targeted at /bin/sh – which is not the case here.
>
> > if [[ -f ${user_manifest_file} ]] || [[ -f ${system_manifest_file} ]]; then
> > if [[ -f ${cache_file} ]]; then
> > # if either one of the manifest files is newer then the cache:
> > if [ ${user_manifest_file} -nt ${cache_file} ] || [ ${system_manifest_file} -nt ${cache_file} ]; then
> > (( 1 ))
> > else
> > (( 0 ))
> > fi
> > else
> > (( 1 ))
> > fi
> > else
> > (( 1 ))
> > fi
I've blindly took the (( 1 )) from a cache policy function I saw on the
documentation.. It's like a return status write?
>
> I find this (( 1 )) stuff confusing. If I'm not mistaken the whole thing
> is equivalent to:
>
> [[ ( ! -f ${user_manifest_file} && ! -f ${system_manifest_file} ) ||
> ! -f ${cache_file} || ${user_manifest_file} -nt ${cache_file} ||
> ${system_manifest_file} -nt ${cache_file} ]]
>
> As for the logic, I'd mainly question what happens when one but not both
> of the manifest files is found to not exist.
The idea with the 1st condition that tests the existence of the
manifests files, is that if non of them exists (which usually never
happens if luarocks is installed properly), is that only one of them is
needed for continuing with the modification date of them vs the cache
file..
Now that I'm thinking about it, I think it would be preferred to improve
the readability of the logic behind this procedure and use `return 0`
instead of `(( 1 ))` and `return 1` instead of `(( 0 ))`. In addition, I
think translating the whole thing to something like the equivalent you
wrote won't help either for readability so I was thinking about something
like this:
local cache_status=1
if [[ -f ${cache_file} ]]; then
if [[ -f ${user_manifest_file} ]]; then
if [[ ${user_manifest_file} -nt ${cache_file} ]]; then
cache_status=0
fi
fi
if [[ -f ${system_manifest_file} ]]; then
if [[ ${system_manifest_file} -nt ${cache_file} ]]; then
cache_status=0
else
cache_status=1
fi
fi
fi
return cache_status
Better?
>
> Note that && and || can be used inside [[ ... ]].
Got it.
>
> Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-05 15:41 ` Doron Behar
@ 2018-06-07 15:59 ` Oliver Kiddle
2018-06-07 16:20 ` Doron Behar
0 siblings, 1 reply; 14+ messages in thread
From: Oliver Kiddle @ 2018-06-07 15:59 UTC (permalink / raw)
To: zsh-workers; +Cc: Doron Behar
Doron Behar wrote:
> The idea with the 1st condition that tests the existence of the
> manifests files, is that if non of them exists (which usually never
> happens if luarocks is installed properly), is that only one of them is
> needed for continuing with the modification date of them vs the cache
> file..
Okay, that makes sense.
> local cache_status=1
> if [[ -f ${cache_file} ]]; then
> if [[ -f ${user_manifest_file} ]]; then
> if [[ ${user_manifest_file} -nt ${cache_file} ]]; then
> cache_status=0
> fi
> fi
> if [[ -f ${system_manifest_file} ]]; then
> if [[ ${system_manifest_file} -nt ${cache_file} ]]; then
> cache_status=0
> else
> cache_status=1
As you've initialised cache_status to 1, this else branch is
superfluous.
> fi
> fi
> fi
> return cache_status
>
> Better?
Yes. Much more readable.
I look forward to the final function.
Oliver
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/1] Squashed commit of the following:
2018-06-07 15:59 ` Oliver Kiddle
@ 2018-06-07 16:20 ` Doron Behar
0 siblings, 0 replies; 14+ messages in thread
From: Doron Behar @ 2018-06-07 16:20 UTC (permalink / raw)
To: zsh-workers
On Thu, Jun 07, 2018 at 05:59:59PM +0200, Oliver Kiddle wrote:
> Doron Behar wrote:
> > The idea with the 1st condition that tests the existence of the
> > manifests files, is that if non of them exists (which usually never
> > happens if luarocks is installed properly), is that only one of them is
> > needed for continuing with the modification date of them vs the cache
> > file..
>
> Okay, that makes sense.
>
> > local cache_status=1
> > if [[ -f ${cache_file} ]]; then
> > if [[ -f ${user_manifest_file} ]]; then
> > if [[ ${user_manifest_file} -nt ${cache_file} ]]; then
> > cache_status=0
> > fi
> > fi
> > if [[ -f ${system_manifest_file} ]]; then
> > if [[ ${system_manifest_file} -nt ${cache_file} ]]; then
> > cache_status=0
> > else
> > cache_status=1
>
> As you've initialised cache_status to 1, this else branch is
> superfluous.
>
Correct, all of these ones and zeros negativity probably confused me..
> > fi
> > fi
> > fi
> > return cache_status
> >
> > Better?
>
> Yes. Much more readable.
>
> I look forward to the final function.
>
> Oliver
Sending right away.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2018-06-07 16:20 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-26 16:14 [PATCH 0/1] *** Add luarocks completion *** doron.behar
2018-05-26 16:14 ` [PATCH 1/1] Squashed commit of the following: doron.behar
2018-05-28 10:48 ` Oliver Kiddle
2018-05-29 15:38 ` Doron Behar
2018-05-29 22:00 ` Oliver Kiddle
2018-05-30 12:47 ` Doron Behar
2018-05-30 15:43 ` Oliver Kiddle
2018-06-01 7:18 ` Doron Behar
2018-06-01 13:30 ` Doron Behar
2018-06-01 22:45 ` Oliver Kiddle
2018-06-03 21:46 ` Daniel Shahaf
2018-06-05 15:41 ` Doron Behar
2018-06-07 15:59 ` Oliver Kiddle
2018-06-07 16:20 ` Doron Behar
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).