dotnet
Alignment Refactoring ordering should be stable
Currently, the various wrapping refactorings use a most-recently-used sorting, that resets on restart. This is awful user experience, and means that I have to reread the entire list every time I want to invoke a refactoring. I don't just use one, I use different styles depending on whether I'm refactoring method parameter lists, or invocations, or line length.
/cc @sharwell @CyrusNajmabadi.
I'm fine with this as long as one of the following holds in order to meet the needs of StyleCop Analyzers users (for whom only one of the wrapping options is valid):
- Indent all parameters is first
- Indent all parameters can be configured to be first via .editorconfig
- All options other than Indent all parameters can be removed from the list via a .editorconfig option (this outcome implies (2) as well)
- The refactoring can be disabled via a .editorconfig setting, a ruleset file, or some other project-specific mechanism
Option (1) is the easiest to implement. Option (3) provides the best overall experience for StyleCop Analyzers users. Option (4) allows the StyleCop Analyzers project to provide their own refactoring to achieve the same final behavior as option (3).
I'm fine with the ordering being persisted across sessions. it was done this way for expediency, but a PR to add persistence around the MRU is not a problem with the design.
I'm fine with the ordering being persisted across sessions. it was done this way for expediency, but a PR to add persistence around the MRU is not a problem with the design.
This will not solve the issue. Which one I use is dependent on current context.
This will not solve the issue. Which one I use is dependent on current context.
Current context is part of the ordering. i.e. we preserve order for invocation wrapping, different from binary wrapping, different from parameter wrapping, etc.
A suggestion from @JoeRobich on Teams:
Wonder if the last one used could be a top level option and all options are available nested with the same order everytime
That would be fine with me and would address the issue.
Current context is part of the ordering. i.e. we preserve order for invocation wrapping different from binary wrapping.
Current context includes current line length.
Current context includes current line length.
Sure. But that can be added to the feature if you want. i.e. "the set and order offered when things went beyond line length" and "teh set an order offered when things are not beyond line length". I'd have no problem with that.
The design of this was not meant to be set in stone :) We've already tweaked a bunch and i'm happy for future improvements.
Wonder if the last one used could be a top level option and all options are available nested with the same order everytime
I like that. Def seems like something worth trying out!
But that can be added to the feature if you want. i.e. "the set and order offered when things went beyond line length" and "teh set an order offered when things are not beyond line length".
This approaches levels of complexity no user is going to be able to wrap their heads around. The current behavior already confused me for a good 10 minutes until I figured out what was going on. I'm fine making the top-level option an MRU, as long as the underlying list is a single set order.
This approaches levels of complexity no user is going to be able to wrap their heads around.
We do MRU things like this all the time. In general, it's not a problem because the top item is often the one that users want. I would not reject this out of hand without actually trying.
I'm fine making the top-level option an MRU, as long as the underlying list is a single set order.
Can you clarify this? I would expect context to still come into play. After all, if you're on a parameter list, and you're not on an argument list, we'd a want the most recent param list option first right?
Can you clarify this?
Copied from offline conversation: I guess my thought on that is, I don't particularly want this to try and be especially smart on the underlying list I'm fine if we try to be smart on the top level option But if it didn't get it right on the top level, then I want it to be predictable where the option I'm looking for is
From offline:
-
i love the idea suggested. i.e. make a topmost 'winner' and then order the rest consistently. We shuld def try that.
-
consider persisting to disk so that across sessions this is still remembered.
-
totally be up for rearranging the default order. feel free to optimize to style-cop defaults
-
if someone is exploring ideas in this space, try a few more arrangements, we might learn some interesting stuff from it.
-
ideally come up with some realistic user scenarios to show the pros/cons of the different approaches against.
Fred, for '5' can you just give a list of small examples of what you were doing. i.e. i had a non-long arg list i wanted to wrap like so, then i had a long param list i wanted to wrap lik so. tehn i went here, and i brought up the list and it was in this bad order that confused me.
That way we can actually examine the results of changes to go definitively: yes that seems like it was improved :)
Thanks!
Design meeting notes (11/29): There's multiple avenues to explore here:
- If we keep the MRU approach, we should have the MRU persist across VS sessions
- Look into pinning an option to the top of the list might be a possibility, but have to talk to Editor team
- Consider options to disable refactorings, maybe make into a code style rule?
Not sure of the status of this, but my personal preference here would be to allow default to a single line but any inserted newlines would always align with the first parameter.
Any progress here? Is there a plan to define it in a .editorconfig file?
