cbor field sorting for struct autogen #19
Conversation
whyrusleeping
force-pushed
the
feat/struct-field-sorting
branch
from
77f18ac to
f0e686d
Compare
|
I think I'm mostly happy with this, but would not get rid of the ability to set key sorter modes per specific map type. Also, when you rebase, could you be so kind as to do the |
obj/atlas/structMapAutogen.go
Outdated
| } | ||
| return entry | ||
| } | ||
|
|
||
| const ( | ||
| StructFieldSort_Default = "default" |
There was a problem hiding this comment.
Missing a type for the consts, plz.
Maybe this is actually the same type and enum when talking about either maps or structs?
| switch mach.cfg.MapMorphism.KeySortMode { | ||
|
|
||
| ksm := atlas.KeySortMode_Default | ||
| if mach.morphism != nil { |
There was a problem hiding this comment.
I don't think this branch should ever be reachable. The marshaller always sets it during dispatch.
There was a problem hiding this comment.
I don't think thats true... If i take this out i get panics in tests
obj/marshalSlab.go
Outdated
| case entry.MapMorphism != nil: | ||
| row.marshalMachineMapWildcard.cfg = entry | ||
| case atl.MapMorphism != nil: | ||
| row.marshalMachineMapWildcard.morphism = atl.MapMorphism |
There was a problem hiding this comment.
This line should revert. If there is an explicit entry in the atlas for this type, we should take it. (People will probably write that and be setting the default at the whole atlas scale at the same time incredibly rarely, but still.)
Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping
force-pushed
the
feat/struct-field-sorting
branch
from
995fb5e to
3de7e76
Compare
|
@warpfork Alrighty. Hows that. I even signed the commit off like a good citizen |
defaults are now scoped to the atlas. which is much better. Signed-off-by: Eric Myhre <hash@exultant.us>
Explicit string-sort vs default are both important. Since these defns are valid for both maps and structs, 'default' and 'strings' sort may be distinct. Signed-off-by: Eric Myhre <hash@exultant.us>
Even if we do have the ability to set global defaults. Exceptions to that might be rare, but we have all this infrastructure here already... Signed-off-by: Eric Myhre <hash@exultant.us>
Prefer to use accessor methods so we're at least that step closer to making it immutable. Of course it's not properly immutable, and we sort of give up because golang is not a terribly powerful language and we've already long admitted in this project that we'll do unsafe things for performance. (That said, I also would really like to have some time to invest into benchmarking some of these details again, possibly with the aim of removing more pointer faffery.) Remove a case in a switch that I'm fairly sure didn't belong there -- that switch as a whole is only hit if you're *got* a specific entry of config, so a branch in it that considers using the default config is nonsensical. I'd make that change in a separate commit, but keeping it across this change to an accessor method would make a mess itself. Also, atlas building now always sets the default mapMorphism. So, we can chase down a bunch of unnecessary/redundant nil pointer checks shortly. Signed-off-by: Eric Myhre <hash@exultant.us>
Signed-off-by: dignifiedquire <dignifiedquire@gmail.com>
Signed-off-by: Jeromy <jeromyj@gmail.com>
whyrusleeping
force-pushed
the
feat/struct-field-sorting
branch
from
f94c73f to
84980a6
Compare
|
whoooopwhoops testsss |
This is still a WIP, but I think its heading in the right direction