[augeas-devel] Basic support for multipart key

[Francis Giraldeau]

> I like this extension a lot; I do have a few comments on the proposed
> patch:
> Don't use malloc/calloc/realloc directly - they have all kinds of
> problems; instead use the wrappers from memory.h. 

Ok, right.

> Doing the concatenation in get_key seems a little unclean to me, though
> I think it's ok. Conceptually, concatenation belongs in get_concat and
> get_quant_star - I can't think of a case where doing concatenation in
> the get_key/get_label functions will cause trouble though.

Yeah, but in the actual scheme, the leaf lenses are actualy doing the
stuff. It make sens, because it will work even if the lens is not in a
concat. The get_key and get_label are concatenating on the state->key,
and put has to pop those values. A list may best suite a string in this
case. 


> >      if (ALLOC_N(types, lens->nchildren) < 0)
> > @@ -269,9 +270,14 @@ struct value *lns_make_concat(struct info *info,
> >          return make_exn_value(info, "Multiple stores in concat");
> >      }
> >      if (l1->key && l2->key) {
> > -        return make_exn_value(info, "Multiple keys/labels in concat");
> > +        return make_exn_value(info, "Multiple keys in concat");
> > +    }
> > +    if (l1->label && l2->label) {
> > +        return make_exn_value(info, "Multiple labels in concat");
> > +    }
> > +    if (l1->key && l2->label) {
> > +        return make_exn_value(info, "Label lens following a key lens is not allowed");
> >      }
> 
> I don't think we need to flag this as an error anymore; shouldn't it be
> possible now to say something like
> 
>         let l = [ label "a" . (label "b" . del "x" "x" | label "c" . del "y" "y") ]
>         
> IOW, it should be possible to have concatenations of any number of keys
> and labels, assuming the lenses are unambiguously concatenable.
> 
> As far as I can see, we should be able to get rid of the key flag in
> struct lens altogether.
> 
> I'd also expect that the typechecker now needs to get involved with
> building a ktype from the concatenation/starring of lenses. Before this
> change, for the lens l = l1 . l2, the ktype was either the ktype of l1
> or the ktype of l2, since we required that only one of them could have a
> non-empty ktype.
> 
> Now, we say that ktype(l) = ktype(l1) . ktype(l2); as a consequence, we
> need to check that ktype(l1) and ktype(l2) are unambiguously
> concatenable.

I think it was done already in fact. In lns_make_concat, here is the
typecheck: 

        struct value *exn = typecheck_concat(info, l1, l2);

But, I will look further into atype to make sure we get the right
behavior here with a test case. 

> > +  let lns = label "a" . label "b"
> 
> This should really be a passing test.

Ok. Right. 

> 
> > diff --git a/tests/modules/fail_multipart_order.aug b/tests/modules/fail_multipart_order.aug
> > new file mode 100644
> > index 0000000..97ba828
> > --- /dev/null
> > +++ b/tests/modules/fail_multipart_order.aug
> > @@ -0,0 +1,3 @@
> > +module Fail_multipart_order = 
> > +
> > +    let k1 = [ key /[a]*/ . label "b" ]
> 
> This should also be a passing test.
> 
> David
> 
> 
> _______________________________________________
> augeas-devel mailing list
> augeas-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/augeas-devel
>