Eric Blake <ebl...@redhat.com> writes: > On 07/29/2015 03:19 AM, Markus Armbruster wrote: >>>> Longest line is a bit over 4KiB for me. >>>> >>> >>> If we break up string literals, at least use some indentation to make it >>> obvious that multiple lines merge to a single array entry. For example >>> (after patch 47): >>> >>> ... >>> "{ 'name': ':abr', 'meta-type': 'object', " >>> "'members': [ " >>> "{ 'name': 'device', 'type': ':acg', 'default': null }, " >>> "{ 'name': 'node-name', 'type': ':acg', 'default': null }, " >>> "{ 'name': 'snapshot-file', 'type': ':acg' }, " >>> "{ 'name': 'snapshot-node-name', 'type': ':acg', 'default': null >>> }, " >>> "{ 'name': 'format', 'type': ':acg', 'default': null }, " >>> "{ 'name': 'mode', 'type': ':afo', 'default': null } ] }, " >>> "{ 'name': ... " >> >> Unconventional indentation, but if it helps the reader... > > I'm not a stickler about the particular spacing I used, so much as > demonstrating an idea. Pick any indentation you like; I was just > demonstrating that some well-chosen line breaks, coupled with visual > clues on what belongs together, can help in reading the string literal > in the generated file. > > In fact, doesn't python have a way to pretty-print JSON, and then > post-process the pretty-printed string to add C \" escaping?
Interesting idea, definitely worth a doc search. Prettier output can of course be punted to a followup-patch. >>> And if we don't want sorting, documenting >>> that data is NOT guaranteed to be position-dependent, in spite of being >>> in a JSON array, is a nice touch. >> >> What do you mean by "position-dependent"? > > If qemu 2.5 has { 'struct':'Foo', 'data': { 'b': 'int', 'c': 'int' } }, > then qemu 2.6 adds '*a': 'int' to the end of that list, then either we > guarantee sorting (if you read the members and see 'b' first, then you > know 'a' was not added) or we don't (you must read the entire list to > see if 'a' has been added; and you cannot assume that 'a' will be last > even if it was listed last in the .json file of 2.6); the position in > the .json file need not determine the position in the introspection output. Got it. >>> But, as with enum sorting, actually documenting our choice will help >>> cement the expectations of clients on what they have to do when learning >>> if a parameter was added. >> >> We may want to adopt a consistent rule on sorting stuff. > > To be consistent, the rule would be that either everything is sorted, or > nothing is; and if we choose nothing to be sorted, we are unlikely to > ever want to add sorting in the future. Agree. If sorting everything turns out to be cheap (in complexity; other metrics don't matter much), I'm inclined to sort. >>>> I could shoehorn both views into a single visitor function, by passing >>>> both views, .base + .local_members, and .members. All implementations >>>> will use only one of the views, but it's not immediately obvious which >>>> one. So I chose to have two visitor functions. Matter of taste. >>> >>> I can live with it (documentation that sub-classes should override at >>> most only one of the two visitors might help the cause, though). >> >> Actually, overriding both would be just fine. We just haven't had a use >> for that. > > I guess it's hard to predict why a visitor would want to have both > callbacks called for every object, so we can't outright state that it is > useless. The whole point of a visitor interface is to provide > flexibility in designing a visitor later down the road :)