Re: caching mechanism and Re: PATCH Completion for _yum

[Oliver Kiddle <okiddle@xxxxxxxxxxx>]

"Jun T." wrote:

> >>  if ( [[ ${+_dput_cfhosts} -eq 0 ]] || _cache_invalid dputhosts ) &&
> >>  ! _retrieve_cache dputhosts; then
> >> - which makes no sense to me.

> I guess what is intended by the original code is to be sure to
> regenerate
> the data if the cache is invalid, even if the variable _dput_cfhosts
> already exists in the current zsh.

That's not what that logic does though. Simplify the line to
  if ( blah blah... ) && ! _retrieve_cache dputhosts

and you will see that it only regenerates the cache if _retrieve_cache
fails. In practice the effect is that no cache is ever considered stale.

> > Whether we want to be keeping caches also in a global variable is a
> > different question

> keeping the data in a variable may not be a good idea.

I'm uneasy about it either way. What about someone who wants to set
use-cache to false? It also seems a pity not to keep the variable
between _complete and various runs of _correct/_approximate. Perhaps
_retrieve_cache could install something in comppostfuncs to clear
them. Or it could track them in an associative array with a snapshot
of $SECONDS and expire old ones before adding new ones to limit memory
usage.

Also note that we widely cache completion data in variables without
using the _store_cache mechanism. A search for typeset\ -g will give you
some idea.

Bart wrote:
> Using $curcontext would end up meaning that there are different copies
> of the cache for different paths into the completion.  That's probably
> not what's intended.

No, there's only ever one copy of the cache. There might be different
policies for different contexts (and multiple policy zstyles all
specifying the same default). Someone might want to force an update
for yum remove but accept an old cache for yum info.

> To be honest I've always thought that the design of the caching system
> was unnecessarily convoluted.  It's trying to be object-oriented and

Never much liked it myself, either. And I've tended to ignore it for
functions I've written. It often seems like overkill - only applicable if
regenerating the cache really takes a noticable amount of time.

> the cache says "I think you'll agree the interface is very simple."
> Two rounds of discussion with Sven W. later, ...)

After the initial part of suggesting things be configurable with styles,
it looks like Sven tried to suggest simplifications, specifically:

  if ! _retrieve_cache RPMs _rpms; then
    _rpms=(...)
    _store_cache RPMs _rpms
  fi

Adam's argument against this comes in 12499 but, with reference to a
current _perl_modules, I'm not sure I buy the argument. Even if it has
validity we should be able to allow the common case to be as simple as
Sven suggests.

Sven also stated:
      We decided some time ago that they [completion functions]
      shouldn't set styles themselves, only look them up.

That was apparently ignored.

How about we fold the zstyle lookup into _retrieve_cache and add options
for specifying a default policy and the tag? Use of _cache_invalid could
be deprecated though with some care to allow old usage to still work.
A function for the policy is often not the simplest thing either. How
about allowing a reference file or age or ... ? So usage:

_retrieve_cache [ -t tag ] -p policy_func | -f file | -a age cache_key var...

The lack of a policy would be supported only for backward compatibility.

The cache-policy style would need extending to cover files an ages.
That's a bit messy but a prefix of / o + in the style value could serve
that purpose.

I'd also suggest adopting a _cache_ prefix on all the variables. Should
we perhaps even enforce that in some way?

Another option would be to deprecate the old mechanism completely?

Oliver