Kevin Wolf <kwolf@xxxxxxxxxx> 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".