Skip to content

Bring back longer lines #2437

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

Closed
radix opened this issue Feb 13, 2018 · 14 comments
Closed

Bring back longer lines #2437

radix opened this issue Feb 13, 2018 · 14 comments
Labels
feature-request only-with-option requires a non-default option value to reproduce

Comments

@radix
Copy link
Contributor

radix commented Feb 13, 2018

Since the change to the "heuristics" model of line length limits, I have been unhappy with the line formatting. I can set my max_width to 100, but many lines that are far below that limit get broken up -- often at something like a 1:4 ratio (!) which leads to a huge increase in vertical size of my code.

@nrc commented that "we'll have to see that the demand from users was there" here: #1984 (comment)

so consider this my input into the demand for more flexible configuration.

@nrc
Copy link
Member

nrc commented Feb 13, 2018

Since the change to the "heuristics" model of line length limits

This wasn't really a change we've done this since the very early days. What changed is having a single config option rather than 6 or something. Were you configuring them individually before? Is just turning off the heuristics too coarse an option?

@radix
Copy link
Contributor Author

radix commented Feb 13, 2018

sorry, I guess my history is a bit off. Yes, I used to have to set several config options to increase various line-limits, but now those are gone.

By "turning off heuristics", do you mean setting use_small_heuristics to false? If so, no, that's kind of the opposite of what I want (as I understand it, it breaks even more lines up). Or did you mean something else? that's the only "heuristic"-related option I saw in Configurations.md

@nrc
Copy link
Member

nrc commented Feb 13, 2018

By "turning off heuristics", do you mean setting use_small_heuristics to false?

Yes

I wonder if we need that to be a three-valued option: prefer short lines, use heuristics, and prefer long lines.

@nrc nrc added the only-with-option requires a non-default option value to reproduce label Feb 13, 2018
@radix
Copy link
Contributor Author

radix commented Feb 14, 2018

@nrc that's an interesting idea. Would "prefer long lines" effectively set all heuristic numbers to equal max_width?

@nrc
Copy link
Member

nrc commented Feb 15, 2018

Would "prefer long lines" effectively set all heuristic numbers to equal max_width?

Something like that, yes

@radix
Copy link
Contributor Author

radix commented Feb 15, 2018

@nrc I would probably be able to put together a PR for that. I can do a WIP tomorrow. The bikeshed would be whether the option name use_small_heuristics should change, and what the values should be called.

@nrc
Copy link
Member

nrc commented Feb 15, 2018

That would be great, thank you!

I would probably change the option name to small_heuristics, but I'm not sure what good names for the options might be.

@radix
Copy link
Contributor Author

radix commented Feb 15, 2018

I don't think small_heuristics applies that well, with a tri-state value. I'm thinking something like

line_width: minimum | medium | maximum, where minimum is equivalent to current use_small_heuristics: false, medium is equivalent to use_small_heuristics: true, and maximum is the new option for maximizing line length.

However, line_width seems awfully similar to max_width, so I'm not sure that's a great name, but I think it's better than small_heuristics.

@philipc
Copy link
Contributor

philipc commented Feb 15, 2018

minimum isn't a good name for the setting that disables all heuristics, because it's a mix of making some lines longer and some lines shorter.

@philipc
Copy link
Contributor

philipc commented Feb 15, 2018

Does anyone actually use the current use_small_heuristics: false setting?

@radix
Copy link
Contributor Author

radix commented Feb 15, 2018

@philipc said on #2299:

@radix That's now cramming long if statements and structs on one line, so I'd probably go with the default heuristics still.

To be honest, I'm not sure if even I want this behavior. I still need to browse through my code formatted with my proposed behavior and see what I think of it. It might be that I still just want more fine-grained options for line-length formatting again... But anyway, I'm blocking on #2446 right now, so I'm trying to fix that.

@radix
Copy link
Contributor Author

radix commented Feb 15, 2018

also, thanks for the note about minimum not making sense, I didn't realize that WidthHeuristics::null() returned usize::max_value() for some options!

@nickolay
Copy link

I wonder if we need that to be a three-valued option: prefer short lines, use heuristics, and prefer long lines.

Now that there's use_small_heuristics with Possible values: "Default", "Off", "Max", I'm not sure what this issue is about. The options are far from ideal, as:

...but it seems like everyone actually just wants better heuristics, and there's a clearer #2636 about that.

@scampi
Copy link
Contributor

scampi commented Apr 8, 2019

The initial request is done through the Max value of the use_small_heuristics option. Discussion for improving the heuristics will take place in the other opened issues listed by @nickolay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

No branches or pull requests

6 participants