This kind of annoys me, and now that I've looked at this code a bit I think it shouldn't be too hard to fix. right now we do: namespace mozilla { namespace dom { class RTCPeerConnection; } // namespace dom } // namespace mozilla namespace mozilla { namespace dom { class RTCPeerConnection; } // namespace dom } // namespace mozilla But this would be a little more space efficiently done more like: namespace mozilla { namespace dom { class RTCPeerConnection; class RTCPeerConnection; } // namespace dom } // namespace mozilla

Oh, and also we could delete duplicates. ;)

Yes, please! And sort alphabetically?

I'll work on this after I finish bug 863880. I'll undo the refactorings in that bug, if I'm going to end up rewriting the mechanism anyway for this bug. So we have -- nest together things declared in the mozilla::dom::namespace -- eliminate duplicates -- sort alphabetically Let me know if you think of anything else. I'll also take a look at the headers we generate and see if anything else springs out at me after the above are fixed up.

This was bothering me, so I ended up just fixing it. I tried to come up with reasonable heuristics for adding vertical whitespace. This takes the number of lines in TestCodeGenBinding.h used for forward declarations from 322 down to 31.

Comment on attachment 740107 [details] [diff] [review] clean up forward declarations in codegen This doesn't actually alphabetize.

Comment on attachment 740111 [details] [diff] [review] clean up forward declarations in codegen >+ def listAdd(self, namespaces, name, isStruct=False): I think we should name this _listAdd, since it's not really public API. >+ def build(self, atTopLevel=True): Is it worth having a public build() with no arguments and a _build with the optional arg? >+ for (cname, isStruct) in sorted(list(self.decls))])) I think just sorted(self.decls) should work fine. >+ for namespace, child in sorted(self.children.iteritems()): What's this sorting the items on? I see nothing that defines how tuples actually sort... I think what should happen here is a sort on the first element of the tuple, using an appropriate key= lambda. >+ if not atTopLevel and (len(decls) > 1 or len(self.decls) > 1): Or maybe len(decls) + len(self.decls) > 1? Either way, I guess. r=me with the above dealt with.

> What's this sorting the items on? I see nothing that defines how tuples actually sort... This page says that tuples are compared lexicographically: http://wiki.python.org/moin/HowTo/Sorting/

Ah, excellent.

(In reply to Boris Zbarsky (:bz) from comment #9) > I think we should name this _listAdd, since it's not really public API. Done. > Is it worth having a public build() with no arguments and a _build with the > optional arg? I did that, except that I made the argument to _build non-optional because we pass it everywhere. > I think just sorted(self.decls) should work fine. Done. > Or maybe len(decls) + len(self.decls) > 1? Either way, I guess. Done.

try run: https://tbpl.mozilla.org/?tree=Try&rev=7ddb2c175154