Stefan Fuhrmann wrote on Sat, Nov 05, 2016 at 22:01:03 +0100:
> On 05.11.2016 17:41, Daniel Shahaf wrote:
> >Stefan Fuhrmann wrote on Sat, Nov 05, 2016 at 13:52:49 +0100:
> >>On 31.10.2016 01:45, Daniel Shahaf wrote:
> >>>stefan2_at_apache.org wrote on Sun, Oct 30, 2016 at 23:43:07 -0000:
> >>>>+ dirent: ( rel-path:string kind:node-kind ? ( size:number
> >>>>+ has-props:bool created-rev:number
> >>>>+ [ created-date:string ] [ last-author:string ] ) )
> >>>>+ | done
> >>>
> >>>Why should size,has-props,created-rev be all present or all absent?
> >>>Wouldn't it be better to let each element (other than the first two) be
> >>>optional, i.e.,
> >>>
> >>> dirent: ( rel-path:string kind:node-kind
> >>> ? [ size:number ]
> >>> [ has-props:bool ]
> >>> [ created-rev:number ]
> >>> [ created-date:string ]
> >>> [ last-author:string ] )
> >>>
> >>>? This decouples the protocol from the backend (specifically, from what
> >>>bits the backend has cheaply available).
> >>
> >>Yes, that would be better.
> >>
> >>It would require to either make has_props a tristate
> >>or extending the protocol to allow for optional bools.
> >>
> >>I think we should do the latter because regardless of
> >>whether we have them at the protocol level or not, we
> >>must specify a value in the output data struct at the
> >>receiver side. So, we might as well define it to FALSE
> >>for n/a data.
> >
> >I don't quite follow, sorry. Let me just clarify my idea:
> >
> >We'll use the spec I gave above, with «[ foo:bool ]», which is an
> >optional bool. At the protocol level, that suffices; it is equivalent
> >to a non-optional «foo:tristate», but without needing to add
> >a "tristate" type to the wire protocol. At the API level, we'd provide
> >two parsing options: '3' to parse the optional bool into an
> >svn_tristate_t, and 'B' to parse the optional bool into an
> >svn_boolean_t, converting svn_tristate_unknown to FALSE. Using 'b' to
> >parse an optional error should be a fatal error (an SVN_ERR_ASSERT).
>
> Well, B does not write to a svn_boolean_t but to an uint64.
>
Well, okay. My point was just that the parser function could let its
caller decide how an absent optional bool would be represented, by
specifying one format character for "wire boolean as svn_boolean_t
('absent' maps to FALSE)" and another format character for "wire boolean
as svn_tristate_t".
> >Makes sense? Could you clarify your idea as well?
>
> I was more looking at the parser / writer code and its format
> specification options.
>
> Not allowing b to be part of e.g. an optional struct makes
> the parser API slightly less easy to use. That does not take
> away from the fact that optional bools are effectively tri-
> states and the API user needs to decide how to treat missing
> data by specifying the suitable data format string.
>
The idea behind forbidding 'b' to be optional in the parser function was
to force callers to choose a behaviour for when the boolean was absent.
(As PEP 20 says — "Explicit is better than implicit".)
> >>And while we are at it, we should make vwrite_tuple()
> >>actually support optional tristates and numbers.
> >
> >What do you mean by "optional tristate"? It sounds like you're talking
> >about a «[ foo:tristate ]», which doesn't appear in the current protocol.
> >If the question is, how would one call vwrite_tuple() to generate
> >a tuple specified as «[ foo:bool ]», then I think there are three ways:
> >
> >* vwrite_tuple("()")
> >* vwrite_tuple("(b)", (svn_boolean_t)foo)
> >* vwrite_tuple("3", (svn_tristate_t)bar)
>
> The last one is currently not supported. That is not symmetric
> to the receiver side where "3" is a support format spec.
>
Right. I see now that the writer function already supports "?", so
adding parentheses to the third case would make sense:
vwrite_tuple("(?X)", (svn_tristate_t)baz)
would generate either «( ) » or «( false ) » or «( true ) ».
> Worse, trying to write an optional number using a "(?n)" format
> causes a malfunction today.
>
That's just a bug. The attached patch should fix it (untested).
> Where they make sense, I'd like to support all format specs and
> combinations in the parser and write - even if we might not use
> them right now.
Personally, I wouldn't worry about this too much; we can implement these
things as and when we need them with little effort.
> The only thing we can't support meaningfully
> on the sender side is a "?b" because we don't have a n/a value
> to check for.
So we'd need some new format code which means "take an svn_tristate_t
argument and render it as a wire bool"; something like the "(?X)"
hereinabove.
The catch is that the 'X' format character should be prohibited in the
*non*-optional part of a tuple, since it's supposed to be marshalled as
a bool, and svn_tristate_unknown is then unrepresentable.
Proof of concept attached. (I used 'X' as a placeholder; we can pick
some more suitable format character)
Cheers,
Daniel
Received on 2016-11-06 02:18:13 CET