Condense )\n.unwrap() into a single line

[TheBlueMatt]

eg it'd likely make sense for

        let data = Vec::<u8>::from_hex(
            "02020202020202020202 long line",
        )
        .unwrap();

to just be

        let data = Vec::<u8>::from_hex(
            "02020202020202020202 long line",
        ).unwrap();

at least for cases where the number of )s (and maybe }s) is under some threshold.

[calebcartwright]

Thanks but going to close as a duplicate.

The tl;dr version is that we're planning on an introducing an option that folks could enable to have rustfmt honor the line breaks the developer did/did not originally add between chain elements. However, we're not going to attempt to support forced rewriting rules because of the myriad, highly subjective options folks have requested and would subsequently want.

Basically, this option would allow you to get your desired output formatting by ensuring you wrote/moved the .unwrap() to the same line, but rustfmt would not convert the first snippet to the second snippet for the same reason: honoring the element association written by the developer.

Refs #4306

[TheBlueMatt]

Hmm, I admit that's pretty disappointing. I agree there's a ton of subjective views on these things, though comparing the rustfmt options available today vs what was available years ago leaves me thinking there's fewer and fewer things where I "strongly disagree" with rustfmt's formatting.

This does look very different from #4306 - that's about the chain length specifically, whereas here I'm suggesting a chain length of one is totally fine, but that a line that only contains )s or }s doesn't need to count has a chain in this context. I don't think addressing #4306 would impact this issue at all either way.

[TheBlueMatt]

As a followup, is this something that you'd accept a patch for, but wouldn't otherwise work on, or is it a "hard no" in the sense that its additional code maintinence that would be a pain?

[calebcartwright]

rustfmt's sole scope is to provide an AST driven pretty printer that emits code according to the rules codified in the Rust Style Guide. I gather from your comments and phrasing on linked upstream PRs/issues that you still adamantly disagree with at least some of those rules, but that's really part of the point.

Our core mission is to support taking any arbitrary input and providing an output based on those rules that's independent of whatever style that input was written in. Yes, rustfmt is configurable, but configuration options are secondary to that core mission. It is unequivocally a non-goal have rustfmt support an open ended set of configuration options so that each individual programmer is able to take any arbitrary input and have rustfmt pretty print it in their preferred style.

This is particularly true for chains. The unwrap call in the chain is wrapped because of this rule. I gather this is one of the rules you still strongly dislike, but if we were to implement/accept/support a specific option to give you what you wanted here, then we'd have to do the same for everyone else and their personal preferred rules.

Chains are far too complex with far too many scenarios with far too many personal preferences and there's no way we'd ever be able to support them all. We have some people that want to wrap/unwrap based on the total length, based on the quantity of elements, based on the variant of elements (some even down to varying behavior based on the idents), based on relative factors between adjacent elements, based on aggregated complexity of the entire chain, based on a complexity threshold of any given element in a chain, and on and on and on (not to mention those that want a combination of 2+ of those). The challenge with chains is that whether or not a developer wants a chain to be wrapped/unwrapped typically depends based on a number of subjective, abstract factors or general "feel" and seemingly just about everyone has a strongly held opinion.

Chains have too many subjective tentacles as compared to more binary considerations, like whether or not to use braces, or tabs vs. spaces, and as such, the best we can do is offer a compromise. This is where, and why, #4306 is absolutely relevant because it speaks specifically to our strategy on chain customization, including this compromise option. rustfmt's defaults will of course still be our pretty printing/steamroll formatting according to the style guide, but we can offer an escape hatch that allows users to control line breaks between their chain elements. Yes, the usage of this new option will require contributors/reviewers to have to pay closer attention to this part of their style and open up bikeshedding debates about whether or not a given line break is better/worse, but that's just part of the deal.

I actually think this approach is not only sufficiently flexible but pretty generous in comparison to the formatters in other ecosystems which typically only offer non-configurable steam roll/pretty-printers, or non/minimally configurable options that with no/minimal support for enforcing width limits. I acknowledge some will disagree with that opinion, but it's still going to be our strategy regardless.

[calebcartwright]

As a followup, is this something that you'd accept a patch for, but wouldn't otherwise work on, or is it a "hard no" in the sense that its additional code maintinence that would be a pain?

Hard no for the reasons detailed above

[TheBlueMatt]

This is particularly true for chains. The unwrap call in the chain is wrapped because of this rule

I'm happy to open a discussion there on the tradeoff between context visibility and code scanability and try to move towards an RFC, but it sounds like that too would likely be a resounding no? I tried to keep this issue as limited as possible - sure, I have other feelings on chains but this specific "context-vs-scanability" tradeoff seemed much less controversial, or at least an option that substantially more projects may be interested in than just myself or other projects I work on. I get that there's maybe too many options to do specific things with chains, though.

I actually think this approach is not only sufficiently flexible but pretty generous in comparison to the formatters in other ecosystems which typically only offer non-configurable steam roll/pretty-printers

Right, I can't say I'm farmiliar with other languages, I mostly have a background in C++, where clang-format is highly configurable, and certainly supports generating code that allows for substantially more context visibility.

[calebcartwright]

I'm happy to open a discussion there on the tradeoff between context visibility and code scanability and try to move towards an RFC, but it sounds like that too would likely be a resounding no?

You can certainly open one proposing a breaking change, but yes I'd expect it to be shot down because I don't think there's much of anything new to be argued, just different perspectives/conclusions on the highly subjective topic. At this point i think it's highly unlikely that we'd introduce significant breaking churn across the ecosystem in order to make everyone's code more aligned to someone else's preference.

that allows for substantially more context visibility.

The "allows" verbiage makes me unsure whether I've adequately conveyed what we're doing in rustfmt. rustfmt will allow you to get the end formatting result you've requested. You will be able to have the .unwrap() call on the same line, the option we're going to add will be idempotent over both snippets in your OP.

The only distinction is that we're not going to give you an option that prohibits your first snippet, and which would convert all occurrences of your first snippet to the second.

[TheBlueMatt]

You can certainly open one proposing a breaking change, but yes I'd expect it to be shot down because I don't think there's much of anything new to be argued,

I don't see much about chains in the issue history on that repo, somewhat surprisingly. I assume these conversations were more offline/discord/whatever rather than me just missing them?

The "allows" verbiage makes me unsure whether I've adequately conveyed what we're doing in rustfmt.

Oops, sorry, I was just being lazy with my wording, I did understand your comment, and indeed was seeking something that would force the second, not leave it optional, though its possible optional would also work. In the mean time I'll maybe consider rustfmt + sed to get the desired outcome, especially if we just restrict to the narrow single-line-with-single-)-or-}-case, its just very frustrating for contributors.

[TheBlueMatt]

cc rust-lang/style-team#171

[calebcartwright]

I don't see much about chains in the issue history on that repo, somewhat surprisingly. I assume these conversations were more offline/discord/whatever rather than me just missing them?

That's largely because other than the recurrent attempts to reopen the great tabs vs. spaces debate, most people aren't strictly interested in trying to change the default. What you'll typically see are feature requests in this repo, and/or comments suggestions on other issues. I can't find the issues ATM, but we've had folks submit requests for functionally the same thing you've requested here just with .await or .as_ref as the chain element instead of unwrap

Context is definitely critical, it's just impossible, or at least borderline impossible, to predetermine context ahead of time in a way that's ubiquitous across all codebases. That's why we think it's better (and more practical) for rustfmt to let the developer determine the context that matters in their circumstances

[TheBlueMatt]

That's largely because other than the recurrent attempts to reopen the great tabs vs. spaces debate,

😂 ah religious wars. I have to admit good on you for taking on a project like rustfmt, I have to imagine you get a lot of people like me showing up and trying to proselytize for their particular religion.

I can't find the issues ATM, but we've had folks submit requests for functionally the same thing you've requested here just with .await or .as_ref as the chain element instead of unwrap

Ah makes sense. Heh, in general I'd suggest those also lining up on the same line, hence why I figured this would be something that is a common enough request to maybe think about making it optional.

Context is definitely critical, it's just impossible, or at least borderline impossible, to predetermine context ahead of time in a way that's ubiquitous across all codebases. That's why we think it's better (and more practical) for rustfmt to let the developer determine the context that matters in their circumstances

Yea, that's fair. To be clear, when I was using the term "context" in the above issue, I was literally referring to the total amount of code that fits onto your terminal, or that appears in git diff/git show/etc. IME that not-irregularly leads to substantially improve code readability/reviewability and I've seen many bugs over the years that would have been caught in review had the reviewer examined more context around the line being changed. Hence my suggestion in that issue that code context visibility be a secondary goal, only insofar as it doesn't conflict with code scanability.

[calebcartwright]

joy ah religious wars. I have to admit good on you for taking on a project like rustfmt, I have to imagine you get a lot of people like me showing up and trying to proselytize for their particular religion.

Ha! It comes with the territory, as we developers are certainly a rather opinionated bunch. Though probably also true that I see a higher volume of subjective opinions being argued as if they were objective fact in this project as compared to others.

Such is life with something as personal and evocative as code styling