fix(entity-generator): include all entity options in EntitySchema definitions #5674
Conversation
boenrobot
force-pushed
the
entitySchemaSerialier
branch
2 times, most recently
from
3bca415
to
e0e9530
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5674 +/- ##
========================================
Coverage 99.74% 99.74%
========================================
Files 261 261
Lines 18121 18122 +1
Branches 4437 4213 -224
========================================
+ Hits 18074 18075 +1
Misses 47 47 ☔ View full report in Codecov by Sentry. |
| properties: { | ||
| age: { primary: true, type: 'smallint', autoincrement: false }, | ||
| }, | ||
| properties: { age: { primary: true, type: 'smallint', autoincrement: false } }, |
There was a problem hiding this comment.
ideally we would keep this as it was, the line breaks for the properties are a good idea, but having this inline with the properties object itself feels rather confusing
There was a problem hiding this comment.
Fair, I guess...
I've added a bit of a "hack" whereby if an object has the "Config" symbol added (with whatever value), it will force indents, even if the line could otherwise fit into less than 80 characters. I could make it so that depending on the value, we may instead force no indenting even if the value is over 80 characters (rather than unconditionally force indent), but we probably don't need that anywhere for now.
There's still differences from before, but the differences are now the places where the line was over 80 characters even before - it is now properly accounted for... Except in the top level and properties, where I now force indents.
EDIT: And the other amends after that one were about the trailing array comma... I noticed I had accidentally removed that. Now it's back in.
boenrobot
force-pushed
the
entitySchemaSerialier
branch
2 times, most recently
from
d9158eb
to
c2ae159
Compare
…initions The getEmbeddableDeclOptions() and getEntityDeclOptions() methods from SourceFile are now reused in EntitySchemaSourceFile, thus ensuring they remain in sync. Serialization of EntitySchema options had to be adjusted a bit for this reuse, which results in different decisions regarding whitespace in EntitySchema. Specifically, previously, all options would always be on their own line, while property definitions would always be on their own line each, and then word wrapped at 80 characters each. Now, instead, 80 character limit is applied on the entirety of the entity options, at every level, with the initial call taking into account the line where the object starts. Whenever a level as a whole would be more than 80 characters, each value is put on its own line, and the limit is applied recursively, taking into account the space before the value.
boenrobot
force-pushed
the
entitySchemaSerialier
branch
from
c2ae159
to
189e954
Compare
| entitySchemaOptions.indexes = []; | ||
| entitySchemaOptions.indexes = this.meta.indexes.map(index => { |
There was a problem hiding this comment.
whats the point of the empty array initializer when you override the value on the next line?
There was a problem hiding this comment.
Oops. Oversight from an earlier, never pushed version... I thought I cleaned that up.
tests/features/entity-generator/__snapshots__/ConflictingEntityNames.mysql.test.ts.snap
Show resolved
Hide resolved
The getEmbeddableDeclOptions() and getEntityDeclOptions() methods from SourceFile are now reused in EntitySchemaSourceFile, thus ensuring they remain in sync.
Serialization of EntitySchema options had to be adjusted a bit for this reuse, which results in different decisions regarding whitespace in EntitySchema.
Specifically, previously, all options would always be on their own line, while property definitions would always be on their own line each, and then word wrapped at 80 characters each.
Now, instead, 80 character limit is applied on the entirety of the entity options, at every level, with the initial call taking into account the line where the object starts. Whenever a level as a whole would be more than 80 characters, each value is put on its own line, and the limit is applied recursively, taking into account the space before the value.