On Thu, Apr 28, 2022 at 03:50:55PM +0200, Markus Armbruster wrote:
> Andrea Bolognani <abolo...@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.

> > 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,
with one of the two forms disagreeing with Go naming 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.

> > 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.

-- 
Andrea Bolognani / Red Hat / Virtualization


Reply via email to