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