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.
> My point is that we don't need the wire protocol to have a notion of "a
> missing bool is interpreted as FALSE"; we can leave that decision with
> each individual call to the parser function.
I think we are in agreement that your proposed representation
of dirent does not require a change to the wire protocol.
> 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.
>> This does not effect any existing protocol usage or
>> code as the format patterns of the existing commands
>> do not change. They are represented as "words" in
>> the pre-parsed "item" structure, so even old code
>> receiving an optional boolean would parse them fine -
>> in case we were to add optional booleans to existing
>> command responses in the future.
>>
>
> I'm not sure what you're trying to say. Of course old code will cope
> just fine with optional booleans; from that code's perspective, the
> optional booleans will occur "past the end of a tuple", so it will
> ignore them entirely. The old code just needs to be able to tokenize
> optional bools, so it can skip past them. That's how the protocol is
> designed.
Well, yes. I just wanted to confirm that the wire protocol
parser (tokenizer) implementation is fine and actually supports
the full range of structures that could be expressed in the
wire protocol. So, that won't bail out on optional data
unless it introduces a new type of structural element.
The actual token -> data parser and more so the writer API
side are less complete.
>> 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.
Worse, trying to write an optional number using a "(?n)" format
causes a malfunction today.
> where the third option generates an empty tuple for svn_tristate_unknown
> and a single-element tuple for svn_tristate_false and svn_tristate_true.
Yes, that is how I would like to implement it.
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. 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.
-- Stefan^2.
Received on 2016-11-05 22:01:09 CET