Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface

[Markus Armbruster]

Andrea Bolognani <abologna@redhat.com> writes:

> On Thu, Apr 28, 2022 at 03:50:55PM +0200, Markus Armbruster wrote:
>> Andrea Bolognani <abologna@redhat.com> writes:
>> > One concern that I have is about naming struct members: things like
>> > SpiceInfo.MouseMode and most others are translated from the QAPI
>> > schema exactly the way you'd expect them, but for example
>> > ChardevCommon.Logappend doesn't look quite right.
>>
>> It doesn't look quite right in the QAPI schema, either: @logappend.  If
>> it was @log-append, as it should, then it would get translated to
>> LogAppend, I guess.
>>
>> Fixing up style isn't a code generator's job.
>
> I agree that the generator shouldn't take too many liberties when
> translating names, and specifically should never attempt to figure
> out that @logappend should have been @log-append instead.
>
> What I was thinking of was more along the lines of, can we change the
> schema so that the proper name is available to the generator without
> breaking the wire protocol? Maybe something like
>
>   ##
>   # ChardevCommon:
>   #
>   # @logappend (rename @log-append): ...
>   ##
>
> That way the generator would have access to both information, and
> would thus be able to generate
>
>   type ChardevCommon struct {
>     LogAppend *bool `json:"logappend,omitempty"`
>   }
>
> The wire protocol would still retain the unappealing name, but at
> least client libraries could hide the uglyness from users.

At the price of mild inconsistency between the library interface and
QMP.

We could clean up QMP if we care, keeping around the old names for
compatibility.  See also Kevin's work on QAPI aliases.  Which is much
more ambitious, though.

>> > Same for the various
>> > structs or members that have unexpectedly-capitalized "Tls" or "Vnc"
>> > in them.
>>
>> Examples?
>
> A perfect one is TlsCredsProperties, whose endpoint member is of type
> QCryptoTLSCredsEndpoint.
>
> On the VNC front, we have SetPasswordOptionsVnc but also
> DisplayReloadOptionsVNC.
>
> There's plenty more, but this should be illustrative enough already.
> Capitalization of these acronyms is inconsistent across the schema,

Common issue with camel-cased compounds containing acronyms, because
either way is ugly.

> with one of the two forms disagreeing with Go naming expectations.

Pardon my ignorance: What are Go's expectations?

> In this case we might be able to just change the schema without
> introducing backwards compatibility issues, though? Type names are
> not actually transmitted on the wire IIUC.

Correct!

>> > but maybe
>> > there's room for adding this kind of information in the form of
>> > additional annotations or something like that?
>>
>> We did for enumeration types: 'prefix' overrides the TYPE_NAME prefix.
>> I fear this was a mistake.
>
> This might be an oversimplification, but I think we might be able to
> solve all of these issues with a single annotation in the form
>
>   namespace:word-MLA-other-words
>
> So for example QCryptoTLSCredsEndpoint would be annotated with
>
>   q:crypto-TLS-creds-endpoint
>
> and each generator would have enough information to produce
> identifiers that fit into the corresponding language, such as
>
>   qcrypto_tls_creds_endpoint
>   CryptoTlsCredsEndpoint
>
> or whatever else.
>
> Of course such annotations would only be necessary to deal with
> identifiers that are not already following the expected naming
> conventions and when MLAs are involved.

Pardon my ignorance some more: what are MLAs?