-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Improve the syntax of some if blocks to one line #5976
Conversation
d38a765
to
010e524
Compare
af30edf
to
4504b67
Compare
Change some if blocks to one-liners and other minor syntax improvements
If these edits are really an improvement is highly questionable. Some perhaps, others probably not. Many changes are purely about style and most of the trailing ifs don't improve readability at all. Neither are early returns always useful. |
We don't have to keep all this modiifcations, but some obsiously improve the readability, like replacing if true
this
else
that
end by return this if true
that and if true
this
end by this if true |
No, I don't think these replacements are particularly improving readability. In some places, maybe. But not in general. |
Like @straight-shoota said, many even exceed standard line width of 80 characters, which highly reduces code readability. |
80 characters like PEP8? |
Can we consider at least to transform little conditions to one-liners? This is of course not possible when we do variable affectations inside the condition. Maybe it is possible to do it with the formatter. |
Sorry, but I'm not sure about this. Depending on the case I prefer and if in one line, the body in another line. In fact many times I find the suffix if harder to read. So this PR hinders readability for me. Let's not discuss this kind of code style. If it compiles and the formatter accepts it, it's fine. |
Ok LGTM I will look at the formatter to improve it. By the way I think there should have a rule for this. |
Please let's not add a width limit to the formatter. It's not useful to me. Monitors are bigger now. |
@asterite It's not just about the monitors, our brains are not rly suited for reading such long lines either (at least mine 😉). |
If it's a long line, put a line break where you think it's convenient. The problem is that a formatter puts the end of line where it can, not where it's nice. I'm against that and I won't change my mind, sorry. |
I don't think it's a formatter's job either, yet keeping 80 character line width as reference point is imho a good thing to have. |
@Sija but 80 characters is a guideline. I super often break past 80 characters. We're not going to implement any kind of length metrics in the formatter, that's something for humans to think about. |
Without having the 80 characters limit globally, I was just underlining the fact that some conditions can be better written. |
The only way to know this is to experiment with the formatter to see if the result is nice to be kept or not 😄 I exagerate a bit, but looking a this make me sad. I can't believe there is nothing to do to beautify this: if true
if true
if true
puts "hello"
end
end
end |
@RX14 that's what I meant writing about a reference point, and so I'd leave it as is (a guideline). |
Change some if blocks to one-liners and other minor syntax improvements.