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

Long function signatures should break #21643

Closed
TheOnlySilverClaw opened this issue Jun 4, 2024 · 8 comments · Fixed by #21782
Closed

Long function signatures should break #21643

TheOnlySilverClaw opened this issue Jun 4, 2024 · 8 comments · Fixed by #21782
Labels
Feature Request This issue is made to request a feature. Unit: vfmt Bugs/feature requests, that are related to the `vfmt`, the formatter for V.

Comments

@TheOnlySilverClaw
Copy link

TheOnlySilverClaw commented Jun 4, 2024

Describe the bug

V puts arbitrarily long function signatures on a single line.

Reproduction Steps

Create any long function signature:

pub fn longer_function_name(x u32, y u32, z u32, first_parameter package_name.LongerTypeName, second_parameter package_name.LongerTypeName, third_parameter package_name.LongerTypeName, fourth_parameter package_name.LongerTypeName, fifth_parameter package_name.LongerTypeName, sixth_parameter package_name.LongerTypeName) {
	first_variable := first_parameter.whatever
}

Expected Behavior

Linebreaks are inserted if a function signature exceeds a certain character limit.
Optionally, manually inserted line breaks are left alone.

Current Behavior

No linebreaks are inserted and manually inserted linebreaks are removed by v fmt.

Possible Solution

No response

Additional Information/Context

previously discussed with @medvednikov

V version

0.4.6

Environment details (OS name and version, etc.)

not relevant

Note

You can use the 👍 reaction to increase the issue's priority for developers.

Please note that only the 👍 reaction to the issue itself counts as a vote.
Other reactions and those to comments will not be taken into account.

@TheOnlySilverClaw TheOnlySilverClaw added the Bug This tag is applied to issues which reports bugs. label Jun 4, 2024
@JalonSolov
Copy link
Contributor

My question is: Why?

Long lines were a problem on printouts, but hopefully (almost?) all of us have moved beyond that stage.

I know people are sometimes annoyed by actually having to scroll horizontally, but others are just as annoyed by having to scroll vertically.

The simplest "fix" for this issue is to create a struct for the parameters, and pass the struct instead of all the separate parameters.

struct LongerFunctionNameParams {
    x u32
    y u32
    z u32
    first_parameter package_name.LongerTypeName
    second_parameter package_name.LongerTypeName
    third_parameter package_name.LongerTypeName
    fourth_parameter package_name.LongerTypeName
    fifth_parameter package_name.LongerTypeName
    sixth_parameter package_name.LongerTypeName
}

pub fn longer_function_name(params LongerFunctionNameParams) {
	first_variable := params.first_parameter.whatever
}

@TheOnlySilverClaw
Copy link
Author

Thinking about it, I personally actually don't care much whether V enforces an arbitrary width limit.
Others might expect that, though.

From my perspective, the opposite question would be warranted:
Why remove manually placed line breaks and mess up formatting without a good reason?

@JalonSolov
Copy link
Contributor

v fmt is an opinionated formatter. Same as go fmt before it.

That's why there are no options - you get what you get... and so does everyone else.

This makes it trivial to look at someone else's code and not have your first thought be "Gah! I have to format this before I can make sense of it!"

I could tell you horror stories...

@TheOnlySilverClaw
Copy link
Author

Those are a bunch of non-arguments going in circles.

This looks like horror to me:
Screenshot from 2024-06-04 19-07-18

So I'd have to actively decide to extract it to a struct as a workaround, which renders the whole point of automatic consistency moot.

I can't remember the last time formatting issues actually kept me from understanding others' code.
But unreasonable default formatting makes me not want to use the language enforcing those on my own code.

An in this case, the formatter isn't just not helping, it's actively disrupting readability for no gain I can think of.

@JalonSolov
Copy link
Contributor

JalonSolov commented Jun 4, 2024

When Go's formatter was released, it was simultaneously the most loved AND most hated feature.

It doesn't give you any options - it always formats things the same way.

What "looks good" to one person may be a complete "horror" to another. The only way to make things fair is to always format the same way, regardless of individual preferences.

That said, if you can make a good case for why a particular thing should be formatted differently (and "because I like it better that way" is not good enough), then perhaps you can convince Alex that it's a change that should be made.

Otherwise, love it or hate it, the formatter will continue to act as it does.

@BreathXV
Copy link

BreathXV commented Jun 5, 2024

I do think having long functions should break to be honest, it is a pain when you have to scroll loads horizontally but I do understand it is preference too.

@TheOnlySilverClaw
Copy link
Author

That said, if you can make a good case for why a particular thing should be formatted differently (and "because I like it better that way" is not good enough), then perhaps you can convince Alex that it's a change that should be made.

Otherwise, love it or hate it, the formatter will continue to act as it does.

As I mentioned in both recently opened issues, that's exactly what I did before opening them:

image

@medvednikov
Copy link
Member

Yes, horizontal scrolling is never good.

This has indeed been approved by me.

@spytheman spytheman added Feature Request This issue is made to request a feature. Unit: vfmt Bugs/feature requests, that are related to the `vfmt`, the formatter for V. and removed Bug This tag is applied to issues which reports bugs. labels Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Request This issue is made to request a feature. Unit: vfmt Bugs/feature requests, that are related to the `vfmt`, the formatter for V.
Projects
None yet
5 participants