Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrapping of long function signatures #725

Open
ahoppen opened this issue Apr 24, 2024 · 4 comments
Open

Wrapping of long function signatures #725

ahoppen opened this issue Apr 24, 2024 · 4 comments

Comments

@ahoppen
Copy link
Member

ahoppen commented Apr 24, 2024

Formatting this to 80 columns

func foo(someArgument: Int, andAnother: Int, andYetAnother: Int) -> Array<Substring> {
}

// produces

func foo(someArgument: Int, andAnother: Int, andYetAnother: Int) -> Array<
  Substring
> {}

Which is definitely not the most intuitive way of formatting it. I would have expected

func foo(
  someArgument: Int,
  andAnother: Int,
  andYetAnother: Int
) -> Array<Substring> {}

In a less extreme case, I would also expect the following to wrap the argument list instead of the return type

func foo(someArgument: Int, andAnother: Int, andYetAnother: Int, and: Int) -> Substring {}

// produces

func foo(someArgument: Int, andAnother: Int, andYetAnother: Int, and: Int)
  -> Substring
{}
func foo(someArgument: Int, andAnother: Int, andYet: Int, and: Int) -> Substring {}

// produces

func foo(someArgument: Int, andAnother: Int, andYet: Int, and: Int) -> Substring 
{}
@ahoppen
Copy link
Member Author

ahoppen commented Apr 24, 2024

Synced to Apple’s issue tracker as rdar://126967203

@allevato
Copy link
Member

IIRC, the reason for this was that we wanted to allow tuple return values to break after the open paren, like:

func foo(someArgument: Int, andAnother: Int, andYetAnother: Int) -> (
  element: SomeType,
  element2: SomeOtherType
) {}

So we never grouped around the return type at all. We should probably use more knowledge of the return type's node kind here to determine whether we want to wrap it in a group or not.


In a less extreme case, I would also expect the following to wrap the argument list instead of the return type

This was the original behavior, which someone else at Apple (I think, it was years ago) argued was too aggressive. 🙂 It can be enabled by setting prioritizeKeepingFunctionOutputTogether in the configuration.

@ahoppen
Copy link
Member Author

ahoppen commented Apr 24, 2024

Oh, I didn’t know about prioritizeKeepingFunctionOutputTogether. Nice

@ahoppen
Copy link
Member Author

ahoppen commented Apr 25, 2024

Enabling prioritizeKeepingFunctionOutputTogether for the sourcekit-lsp codebase caused an assertion failure, I filed #726 for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants