kaitai_struct_compiler icon indicating copy to clipboard operation
kaitai_struct_compiler copied to clipboard

#252 - groups the public and private class members.

Open tempelmann opened this issue 8 years ago • 6 comments

Makes the .h file neat and tidy :)

tempelmann avatar Sep 02 '17 12:09 tempelmann

Note that I "abused" the ImportList for collecting the lines. I didn't want to create yet another class with the same functionality.

tempelmann avatar Sep 02 '17 12:09 tempelmann

About the bot warnings: I should probably declare them private.

Also: It's still buggy: Comments (from "doc:") do not generate correctly.

tempelmann avatar Sep 02 '17 12:09 tempelmann

The comments bug comes from the fact that I only add unique lines - a mistake, when it comes to comment lines. Fixing that now...

tempelmann avatar Sep 02 '17 12:09 tempelmann

Alright. Fixed. I ended up not using the ImportList (instead using my own StringList class) and added a stack to properly manage the collector objects.

tempelmann avatar Sep 02 '17 12:09 tempelmann

I can't avoid the mutable warning, though. I have to change the publicDecls between classHeader and classFooter, to handle nesting of classes. No idea how to do that otherwise.

tempelmann avatar Sep 02 '17 12:09 tempelmann

I've reviewed this PR and I believe there are quite a few problems with it:

  • It changes a lot of things not related to the topic and not mentioned anywhere in these commit messages and/or PR text, for example:
  • It adds a new private class StringList, which seems to be completely redundant for me — one can just use ListBuffer[String] instead of it.
  • Two instances of this class are wrapped into Stack for some unknown reason
  • Whole ensureMode system is heavily changed to use stuff like privateDecls.addUnique instead. It's not idempotent replacement, and, besides, addUnique is useless there anyway — these calls generated syntactically correct (i.e. unique) declarations before, so after a change they should be unique anyway.

It its current state it's unlikely to be accepted. I'd suggest that we rework it using existing LanguageOutputWriter buffering classes: we have outHdr, outSrc, etc, now, so it's quite easy to add outHdrPrivate and outHdrPublic, and dump it to main outHdr — as you do — in a class footer.

GreyCat avatar Sep 13 '17 11:09 GreyCat