Re: [PATCH 1/1] Squashed commit of the following:

[Doron Behar <doron.behar@xxxxxxxxx>]

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