Discretionary line break overridden

[dabrahams]

Description

Even though I have

  "respectsExistingLineBreaks" : true,

in my configuration, swift format undoes the line break in this case (and de-indents the where clause)

public func sourceFiles<S: Collection>(in sourcePaths: S) throws -> [SourceFile]
  where S.Element == URL
{
  return try sourceFilePaths(in: sourcePaths).map(SourceFile.init)
}

Steps to Reproduce

No response

[allevato]

respectsExistingLineBreaks still removes line breaks that are considered invalid by the style guidelines that the formatter implements. In this case, since the encoded guidelines indicate that where clauses are aligned +0 from their declaration, the { is placed on the end of that same line (when it fits).

So this is working as intended for now. I'm willing to consider configuration options for different styles but I'm also hesitant right now when it comes to things that affect declaration line-wrapping since the current implementation doesn't scale well as the number of options increases. I have some thoughts on how to improve this going forward, but I haven't had as much time as I'd like to prototype it out.

[dabrahams]

It's not the de-indenting of the where clause that I'm reporting as a bug, but the removal of the discretionary line break.

[allevato]

Do you mean that you would expect this output, given the de-indenting of the where clause?

public func sourceFiles<S: Collection>(in sourcePaths: S) throws -> [SourceFile]
where S.Element == URL
{
  return try sourceFilePaths(in: sourcePaths).map(SourceFile.init)
}

The general philosophy for our current line breaking scheme is that for braced structures, the open brace is on the same line if the indentation is such that the line that would contain the brace is indented less than the lines inside the braces, because there would be no confusion that the latter is a continuation of the former. Only if the line that would normally contain the brace is indented is the brace moved down to its own line, at a reduced indentation level.

Effectively, a line break before the { is not considered discretionary; it is only applied if necessary.

[dabrahams]

On Mar 13, 2023, at 3:45 PM, Tony Allevato ***@***.***> wrote: Do you mean that you would expect this output, given the de-indenting of the where clause?

I don’t understand. Doesn’t that depend on what input and options we give it?

public func sourceFiles<S: Collection>(in sourcePaths: S) throws -> [SourceFile] where S.Element == URL { return try sourceFilePaths(in: sourcePaths).map(SourceFile.init) } The general philosophy for our current line breaking scheme is that for braced structures, the open brace is on the same line

Same line _as what_?

if the indentation is such that the line that would contain the brace is indented less than the lines inside the braces,

OMG that’s complicated. I don’t understand why the input's indentation should ever play a role in line breaking decisions. I am not asking for the ability to make decisions about indentation, BTW.

because there would be no confusion that the latter is a continuation of the former. Only if the line that would normally contain the brace is indented is the brace moved down to its own line, at a reduced indentation level. Effectively, a line break before the { is not considered discretionary; it is only applied if necessary.

I don’t understand again. When I say "allow discretionary line breaks” I mean, "allow the user to break lines earlier than absolutely necessary and don’t rejoin the lines automatically”, where “the line is broken earlier than necessary” means that adding the next token to the end of the line would not violate my rule that unmatched opening delimiters come last on their line (with the exception for closure signatures). So I don’t understand what it means to “consider a line break discretionary…” I guess the only ones I would consider discretionary are those that occur earlier than absolutely necessary per the above defintion.

…

— Reply to this email directly, view it on GitHub <#470 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAKYIL5EUAAHJ3RFPCQ2T3W36PQXANCNFSM6AAAAAATWA2AOQ>. You are receiving this because you authored the thread.

[dabrahams]

This feature again fails to do what I expect. If I write:

if x {
  foo()
}
else {
  bar()
}

It removes the line break before else. I thought that this feature was supposed to implement my long-ago request, when sf was under development, to “allow discretionary line breaks” but now I suppose I was mistaken.

The docs say

Indicates whether or not existing line breaks in the source code should be honored (if they are valid according to the style guidelines being enforced)

Presumably this means “the style guidelines being enforced” include a rule that there are no line breaks between else and a preceding }. How do I find out what these rules are, and how do I change them? I don't want any rules that prohibit line breaks anywhere. That's what I meant by “allow discretionary line breaks.”

[dabrahams]

OK, @allevato, I'm a little sorry to keep hammering on this, but I found documentation of the rules, and there is no mention of anything that prohibits a line break anywhere. What are the actual semantics of respectsExistingLineBreaks?

[allevato]

We don't really have documentation for all the cases; it's based on the prevailing style that we've chosen to implement. Some of the ideas are documented in https://google.github.io/swift/ but swift-format has also drifted slightly since that document was originally written.

Those specific cases can be examined by looking at TokenStreamCreator's implementation; any breaks that don't have newline behavior of .elective(ignoresDiscretionary: true) will permit a newline to be inserted there. However, this might also cause newlines to be inserted elsewhere for symmetry; for example, the following is not allowed, and a newline would be inserted before the closing brace:

if x {
  print() }

I'll be candid though; I'm extremely hesitant when considering new options that would expand the matrix of configurability for the formatter compared to previously. (Tree examining/transforming rules are less risky, but interactions between different pretty-printing knobs can cause very strange issues.) It's been years since SE-0250 was proposed and deferred, and we're still in the same state regarding whether the Swift project would recommend an official style guide and include swift-format in other official tooling (there's an open PR to add it to sourcekit-lsp, which I think is currently blocked on something Windows-related). Decisions like that would inform what to plan for the future of swift-format, and whether it should be more opinionated like gofmt or be a truly general purpose customizable formatter.

As the sole maintainer of the project, my main priority right now is keeping the logic that we currently have implemented working and expanding it to support new Swift syntactic constructs as they are added to the language. Supporting more customization is a far lower priority.

[dabrahams]

Of course you can make whatever choices you want about the design of the project, its functionality, and which patches to accept. The only thing I really feel I have an absolute right to insist on is that the documentation is accurate, complete, and consistent. I would offer to fix the documentation for respectsExistingLineBreaks but after all this back and forth I still don't know what it's supposed to do. Once it's documented, I can decide whether I want to propose a change, fork the project, etc.

[ahoppen]

Tracked in Apple’s issue tracker as rdar://126948320