Condense output in presence of multiple suggestions #43
Conversation
| @@ -38,11 +36,11 @@ try | |||
| [1;34m -->[0m src/controls.rs:373:5-378:6 | |||
|
|
|||
| try this | |||
| [0]: [1;33mSuggestion - Replace:[0m | |||
| [0]: [1;33mAt:[0m | |||
| [31m⍉[0mpub fn new() -> Entry { | |||
There was a problem hiding this comment.
Fun bug, but this is a clippy bug, so I won't fix it here. Clippy should be placing this suggestion properly.
src/main.rs
Outdated
| // check whether we can squash all suggestions into a list | ||
| if solution.replacements.len() > 1 { | ||
| let first = solution.replacements[0].clone(); | ||
| if solution.replacements.iter().all(|s| first.snippet.file_name == s.snippet.file_name && first.snippet.line_range == s.snippet.line_range) { |
There was a problem hiding this comment.
Can you put this in a let all_suggestion_are_alternatives_to_each_other = … (or something shorter)? Then, this line becomes the much more readable if all_suggestion_are_alternatives_to_each_other {.
src/main.rs
Outdated
| let first = solution.replacements[0].clone(); | ||
| if solution.replacements.iter().all(|s| first.snippet.file_name == s.snippet.file_name && first.snippet.line_range == s.snippet.line_range) { | ||
| prelude(&first); | ||
| for suggestion in solution.replacements.iter() { |
There was a problem hiding this comment.
Didn't clippy suggest &solution.replacements here?
There was a problem hiding this comment.
I think clippy throws up all kinds of things when used on rustfix. I never used it ;) I should. And I should feel bad for not having it done yet. ... Ah now I remember I wanted to fix rustfix with rustfix, but some clippy suggestions were breaking it. So I wanted to fix those first. I'm probably trapped in a loop
src/main.rs
Outdated
| println!( | ||
| "{lead}{text}{tail}", | ||
| lead = indent(4, &snippet.text.0), | ||
| text = "⍉".red(), |
There was a problem hiding this comment.
What's up with the apl functional symbol circle backslash here? Do you mean to convey that the text is empty here? Then: Did you mean to use the empty set—or actually ε?
There was a problem hiding this comment.
I just wanted a pretty symbol. I would have preferred something like the google maps "you are here" marker. You know. with the pointy bottom and the round top.
There was a problem hiding this comment.
idc. If you have sth you like, I'll use it.
There was a problem hiding this comment.
It's probably best to use something people actually have in their terminal font… so, v or ^? I don't mind either way
There was a problem hiding this comment.
if you want to change it, go ahead, otherwise i'm fine with merging with backslashed circle.
There was a problem hiding this comment.
I'll change it when I get back to a PC. Github web editing on mobile is a pain and would produce many commits ;)
src/main.rs
Outdated
| fn prelude(suggestion: &Replacement) { | ||
| let snippet = &suggestion.snippet; | ||
| if snippet.text.1.is_empty() { | ||
| if suggestion.replacement.ends_with('\n') || suggestion.replacement.starts_with('\n') { |
There was a problem hiding this comment.
What are you testing here? What meaning does a replacement starting or ending with a newline have? Can you maybe rewrite this as let meaningful_name_of_usecase_here = …; if meaningful_name_of_usecase_here {?
There was a problem hiding this comment.
öhm... the use case is suggestions which wanna be on their own line... by being inserted after or before another line.
There was a problem hiding this comment.
Doh! Okay, that is pretty obvious…
-ly to be named let wants_to_be_on_own_line :D
| println!("{}", "At:".yellow().bold()); | ||
| println!( | ||
| "{lead}{text}{tail}", | ||
| lead = indent(4, &snippet.text.0), |
There was a problem hiding this comment.
Memo for later refactoring: Turn text from (String, String, String) into struct ReplacementText { leading: String, content: String, trailing: String, }
|
Thanks! bors r+ |
Build succeeded |
part of #41