Reduce line-length to 160 characters #1714
Conversation
|
@swift-ci Please test |
There was a problem hiding this comment.
A more general comment:
Some places methods are split after the last parameter so ) -> someReturnValue is on a new line.
That looks a bit odd to me and doesn't read that well IMHO.
Same for choices in Nodes where some elements of the list is on a newline and others are not.
| .token(tokenKind: "IdentifierToken"), .token(tokenKind: "BinaryOperatorToken"), .keyword(text: "init"), .keyword(text: "self"), | ||
| .keyword(text: "Self"), |
There was a problem hiding this comment.
Would it make sense to add a newline for all these .token() as it looks a bit odd to me to only have the last on a newline?
There was a problem hiding this comment.
I don't have a strong opinion on whether each should be on a newline or there should be lines of multiple tokens, but it is odd that the format will format any past 160 onto a line per token 🤔
There was a problem hiding this comment.
@bnbarham, that was an artifact of how I kept reducing the line length to see the effect and then just re-ran swift-format, which respected existing line breaks.
I changed these long lists to be on new lines now. I agree that it makes scanning the lists easier.
| @@ -322,7 +325,8 @@ public let ATTRIBUTE_NODES: [Node] = [ | |||
| kind: .derivativeRegistrationAttributeArguments, | |||
| base: .syntax, | |||
| nameForDiagnostics: "attribute arguments", | |||
| documentation: "The arguments for the '@derivative(of:)' and '@transpose(of:)' attributes: the 'of:' label, the original declaration name, and an optional differentiability parameter list.", | |||
| documentation: | |||
There was a problem hiding this comment.
Is there a swift-format option for making the string multiline?
There was a problem hiding this comment.
Unfortunately, there isn’t. Really these doc comments should just be multi-line strings so that the generated documentation also has newlines to improve readability. But if we want to change that, I’d like to do it in a follow-up PR.
ahoppen
force-pushed
the
ahoppen/line-length
branch
from
332eb9a to
8a5738a
Compare
Sources/SwiftParser/Specifiers.swift
Outdated
| mutating func at<SpecSet1: TokenSpecSet, SpecSet2: TokenSpecSet>(anyIn specSet1: SpecSet1.Type, or specSet2: SpecSet2.Type) -> ( | ||
| spec: TokenSpec, handle: TokenConsumptionHandle, matchedSubset: Any.Type | ||
| )? { |
There was a problem hiding this comment.
Could this also be on a newline to match the style of returns?
ahoppen
force-pushed
the
ahoppen/line-length
branch
from
8a5738a to
45b8578
Compare
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/line-length
branch
3 times, most recently
from
8dcd599 to
f796dfe
Compare
|
@swift-ci Please test |
I played around withs some maximum line lengths and 160 seems to strike a good balance of prohibiting excessivly long lines while not forcing line wraps on lines that are more clearly represented on a single line.
ahoppen
force-pushed
the
ahoppen/line-length
branch
from
f796dfe to
9626030
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
I played around withs some maximum line lengths and 160 seems to strike a good balance of prohibiting excessively long lines while not forcing line wraps on lines that are more clearly represented on a single line.
This was motivated by swiftlang/swift#66044 (comment). To repeat some of the data points from that comment.
Here’s a chart of the line lengths in non-generated SwiftSyntax code.
Some key data points: