50 keep new lines in arrays #85
Conversation
The PPWindowFormatter formats Strings so that in one line there are no more than maxLineWidth Characters. One exeption to this is if you have a Word which has more Characters than maxLineWidth.
66 comma and spaces
Test Case for Space before Comma in Array added
fully solving issue 66 option to set space before comma and space before dot in Array
Rebase Release_Branch onto master
…ndard-browser-window 49 set row length to standard browser window
…swa-teaching/poppy-print.git into 50-keep-new-lines-in-arrays
There was a problem hiding this comment.
Functionality works as expected, great stuff! I added a couple style comments.
I specifically tried to ignore all changes related to the windowformatting. Would be great to have this separated so that it doesn't end up in master prematurely, but since it won't actually be used I'm also okay with merging as-is.
| braceNodeHasMultiLineParts: aNode | ||
|
|
||
| ^ (aNode elements anySatisfy: [:node | | ||
| (self willBeMultiLine: node) or: (self isTypeOfNewLineMarker: node)]) or: [self isCaseOf: (self parentFor: aNode)] |
There was a problem hiding this comment.
missing square brackets around the or:
| result := aBoolean | ||
| ifTrue: [self stripMethodPattern: formatter contents] | ||
| ifFalse: [formatter contents]. | ||
| aPPFormatterConfig formatToMaxLineWidth ifTrue: [result := PPWindowFormatter new format: result withWindowWidth: aPPFormatterConfig maxLineWidth]. | ||
|
|
||
| ^ result |
There was a problem hiding this comment.
I guess this was used during debugging only? or is there a reason to keep the temp var around?
There was a problem hiding this comment.
no, this is actually needed for the LineWidth
...ges/PoppyPrint-Core.package/PPFormatter.class/instance/evaluateBraceStatements.multiLine..st
Outdated
Show resolved
Hide resolved
| ((self isEmptyLineMarker: element) or: [self isOptionalEmptyLineMarker: element]) | ||
| ifTrue: [self newLine] | ||
| ifFalse: [ | ||
| (self isOptionalNewLineMarker: element) ifFalse: [ |
There was a problem hiding this comment.
could be that I'm reading this wrong, but isn't this always false because if it were true, control flow would have gone into the upper ifTrue branch?
One suggestion, to help clarify the method, could be to create a separate printBraceElement:multiLine: method and make use of early returns.
| helper | ||
| isTypeOfNewLineMarker: aNode | ||
|
|
||
| ^ ((self isOptionalEmptyLineMarker: aNode) or: (self isOptionalNewLineMarker: aNode) or: (self isEmptyLineMarker: aNode)) |
There was a problem hiding this comment.
missing square brackets around or
packages/PoppyPrint-Core.package/PPFormatter.class/instance/visitBlockNode..st
Show resolved
Hide resolved
packages/PoppyPrint-Core.package/PPFormatter.class/instance/visitBraceNode..st
Show resolved
Hide resolved
| @@ -3,23 +3,39 @@ codeWithEmptyLineMarkersStartingAt: aNumber | |||
| " insert #ppEmptyLine symbols where double newlines are found, for later use by e.g. a formatter. Insert no markers earlier than aNumber " | |||
There was a problem hiding this comment.
consider also updating the method comment
Solve Issue 50. (Contains changes from Issue 49)