Respect file-lines range per predicate in where clauses#6874
Respect file-lines range per predicate in where clauses#6874Souradip121 wants to merge 6 commits intorust-lang:mainfrom
Conversation
Previously rewrite_bounds_on_where_clause and the Visual-indent path in rewrite_where_clause reformatted every predicate unconditionally, causing the entire where clause to change even when only one predicate's lines were selected via --file-lines. Now each predicate is checked against the file-lines range before rewriting. Predicates outside the range fall back to their original source text; predicates inside the range are formatted normally. Fixes rust-lang#6872
rustbot
added
the
S-waiting-on-review
| |pred| { | ||
| if out_of_file_lines_range!(context, pred.span()) { | ||
| Ok(context.snippet(pred.span()).to_owned()) | ||
| } else { | ||
| pred.rewrite_result(context, shape) | ||
| } | ||
| }, |
There was a problem hiding this comment.
Instead of doing this check here, I'm wondering if we could move this logic to the Iterator impl for ListItems. It will likely require us to pass the whole RewriteContext to itemize_list instead of just the snippet_provider, but that should be fine.
Moving this check there means that we wouldn't need to duplicateif out_of_file_lines_range! every time we use itemize_list, and it means that you'd actually fix things for other code paths that use this pattern.
There was a problem hiding this comment.
Alright, made a commit, kindly check.
Thread &RewriteContext through itemize_list instead of just &SnippetProvider, then check out_of_file_lines_range! once per item inside Iterator::next(). This covers all call sites automatically rather than requiring per-closure handling at each call site. Revert the per-predicate closures in rewrite_where_clause back to simple one-liners now that the iterator handles it centrally.
|
This PR does not fully address #6872. This only seems to address inline spacing within a single predicate (i.e. the spacing between the type param and the bound |
| where | ||
| T: Clone + Debug, | ||
| U: Copy, | ||
| V: Default, |
There was a problem hiding this comment.
A minor note: The way this test is written it's a bit hard to see what exactly is changing between the source file and target file, since only a single space is changing. I'd suggest changing the input file so that the bad formatting is more obvious, such that it's easier to tell at a glance what the test covers:
fn foo<T, U, V>()
where
T : Clone + Debug,
U : Copy,
V : Default,Would then become
fn foo<T, U, V>()
where
T: Clone + Debug,
U : Copy,
V : Default,
|
Thanks for the catch, just updated the test per your suggestion, the diff reads much clearer now. Also pushed a bailout so the whole where clause stays untouched when none of its lines are in the selected range, which fixes the where-keyword-moves issue for that case. The partial case (some predicates in range, others out, and the where line itself out) needs line-level source splicing and is more invasive also happy to tackle that as a follow-up, or fold it in here if you'd prefer. |
| return Ok(String::new()); | ||
| } | ||
|
|
||
| if !context.config.file_lines().is_all() && out_of_file_lines_range!(context, where_span) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Okayy, just created a commit applying it.
I think that'd be reasonable to split out into its own PR. Now that this PR is generalized to handle all lists, I think this makes sense to land on its own first, since this will also help other issues like #6868. |
4 participants
Solves #6872
Summary
rewrite_bounds_on_where_clause(Block/RFC indent path) and the Visual-indent path inrewrite_where_clauseboth unconditionally rewrote every predicate, causing the entire where clause to be reformatted even when only a subset of predicate lines were selected via--file-linesout_of_file_lines_range!per predicate inside theitemize_listclosure — predicates outside the selected range now fall back to their original source text, predicates inside are formatted normallysrc/expr.rs,src/stmt.rs, andsrc/visitor.rstests/source/file-lines-where-clause.rs+tests/target/file-lines-where-clause.rsto cover the partial-formatting caseTest Plan
cargo testpassesfile-lines-where-clauseverifies that only the predicate on the selected line is reformatted; out-of-range predicates retain their original textfile-lines-*tests continue to pass (no regression in full-range formatting)