Re: [PATCH 01/18] qapi: Smarter camel_to_upper() to reduce need for 'prefix'

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 30.07.2024 um 10:10 hat Markus Armbruster geschrieben:
>> camel_to_upper() converts its argument from camel case to upper case
>> with '_' between words.  Used for generated enumeration constant
>> prefixes.
>> 
>> When some of the words are spelled all caps, where exactly to insert
>> '_' is guesswork.  camel_to_upper()'s guesses are bad enough in places
>> to make people override them with a 'prefix' in the schema.
>> 
>> Rewrite it to guess better:
>> 
>> 1. Insert '_' after a non-upper case character followed by an upper
>>    case character:
>> 
>>        OneTwo -> ONE_TWO
>>        One2Three -> ONE2_THREE
>> 
>> 2. Insert '_' before the last upper case character followed by a
>>    non-upper case character:
>> 
>>        ACRONYMWord -> ACRONYM_Word
>> 
>>    Except at the beginning (as in OneTwo above), or when there is
>>    already one:
>> 
>>        AbCd -> AB_CD
>
> Maybe it's just me, but the exception "at the beginning" (in the sense
> of "after the first character") seems to be exactly where I thought
> "that looks strange" while going through your list below.

By "except at the beginning", I mean don't map "One" to "_ONE".

>                                                           In particular,
> I'd expect X_DBG_* instead of XDBG_*.

What's the intent of the X in the XDbgFOO types?  Signify unstable?

If yes: we don't do that elsewhere.  Type names are not part of the
external interface.  We never used an X prefix for names of unstable
types.  We use an x- prefix for names of unstable commands, arguments
and members, but even that is optional today.  Feature flag @unstable is
the source of truth.

The XDbgFOO appear to be used just by x-debug-query-block-graph, which
has feature @unstable.

If the XDBG name bothers you, we can strip the X prefix from the type
names.  Happy to do that in this series.

>                                       I also thought that the Q_*
> spelling made more sense, though this might be less clear.

The crypto subsystem spells its prefix qcrypto_, QCRYPTO_, and QCrypto.
Before this series, it forces QAPI to generate QCRYPTO_ with 'prefix'
with two exceptions, probably oversights.

>                                                            But in case
> of doubt, less exceptions seems like a good choice.

Agree.  I want to be able to predict generated names :)

>> +    # Copy remainder of ``value`` to ``ret`` with '_' inserted
>> +    for ch in value[1:]:
>> +        if ch.isupper() == upc:
>> +            pass
>> +        elif upc:
>> +            # ``ret`` ends in upper case, next char isn't: insert '_'
>> +            # before the last upper case char unless there is one
>> +            # already, or it's at the beginning
>> +            if len(ret) > 2 and ret[-2] != '_':
>> +                ret = ret[:-1] + '_' + ret[-1]
>
> I think in the code this means I would have expected len(ret) >= 2.

I'm not sure I understand what you mean.

With len(ret) > 2, we map "QType" to "QTYPE".

With len(ret) >= 2, we'd map it to "Q_TYPE".