Re: PATCH: completion caching layer

[Adam Spiers <adam@xxxxxxxxxx>]

Adam Spiers (adam@xxxxxxxxxx) wrote:
> Sven Wischnowsky (wischnow@xxxxxxxxxxxxxxxxxxxxxxx) wrote:
> > 
> > Adam Spiers wrote:
> > 
> > > Sven Wischnowsky (wischnow@xxxxxxxxxxxxxxxxxxxxxxx) wrote:
> > > > It's a bit unfortunate that _cache_invalid can be called twice (in
> > > > your examples), once directly and once from _retrieve_cache. I think.
> > > 
> > > Yes, I didn't like that either, but couldn't think of a better
> > > design.  The problem is that there are actually two caching layers -
> > > the parameters, and the cache files on disk, but the _cache_invalid
> > > check needs to be invoked if either is about to be used.  Suggestions
> > > for how to avoid this welcome.
> > 
> > Couldn't we stuff everything in _retrieve_cache? So that one only
> > needs to call:
> > 
> >   if ! _retrieve_cache RPMs _rpms; then
> >     _rpms=(...)
> >     _store_cache RPMs _rpms
> >   fi
> 
> I had thought of that, and then decided against it, probably for no
> good reason whatsoever, as I was exhausted at the time :-) 

I remembered the reason, and why I thought it might be good.  I was
thinking that the problem with moving the parameter test into
_retrieve_cache is that the completion function programmer loses
control over it, and while in the majority of the situations the test
will be of the nature 'check if the parameter is undefined, and if so
we need to retrieve from the cache file', there could be situations in
the future where the parameter would correctly end up undefined
/after/ retrieval from disk.  For example, suppose that _perl_modules
stored the results of its search for perl modules in several
parameters, instead of one:

  _perl_pragmatic_modules
  _perl_standard_modules
  _perl_extension_modules
  _perl_CPAN_modules

(See the perlmodlib man page for an explanation of these.)

Now, it's not hard to imagine a system which doesn't have any CPAN
modules installed.  In that case, I was worried about the above test
failing because _perl_CPAN_modules would be undefined.  However there
appears to be an easy solution, which is just to ensure that all
cache-related parameters are always defined after retrieval, even if
they're defined to the empty array or string.  Does that sound OK?