-
Notifications
You must be signed in to change notification settings - Fork 4k
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
C# - .editorconfig option for force trailing comma #25206
Comments
Is this still being considered? Was thinking about writing a rule to check for this, but if this is the right repo in which to implement this I'd be interested in submitting a PR here instead. |
Additional context from @AArnott There is a StyleCop.Analyzer that already does this: But it would be nice to add it to .editorconfig as well so that code generators can respect it. |
Design review conclusion: it makes sense to add an editorconfig option to include this comma, which the various code generation features would then account for. We would also accept a pull request implementing an analyzer and code fix for this option, but the main item to implement first is the editorconfig option which is used by the syntax generator for code fixes and refactorings. |
A year later, did this get anywhere? |
@arch-stack this item has not been implemented so far. It's still up-for-grabs if someone in the community is available to contribute the implementation, but we don't currently have it scheduled to complete ourselves. |
Looking at some examples of analyzers and code fixes you have, I'm interested. Seems the idea is to write an abstract base class that does the basic work, then pass in whatever language-specific tokenizers, etc., are needed. Though, in this case, after some doc lookups and experimentation, VB doesn't support trailing commas on enums (well, no commas anyway), collection initializers, or object initializers (the latter of which uses |
Yes, this would only be needed/implemented for C#. |
I'd be interested in trying to figure this out but it's a huge uphill learning experience for me. Wouldn't even know where to start |
@arch-stack I'd recommnd starting with something smaller first just to get a hang of the basics of roslyn (i.e. enlisting, building, modifying, testing, pull-requests, etc). When comfortable there, moving to something like this would be a lot easier :) |
@arch-stack this is a little more challenging than some of our "first time" fixes, but probably still can be done. I would expect the following sequence would be the most successful:
Once these three steps are done, we have the option of either merging the change as a first step and/or providing guidance on which feature(s) should be tackled next. |
I've got a start on this but wondering if we want the granularity of separate options per enum, object initializer, and collection initializer. I'm assuming so for now since existing options (related or otherwise) are granular. Or just a single option like UseTrailingCommasInMultiLineInitializers? |
With formatting options we generally separate out expressions versus declarations. So I would do the same here. Two options. One for trailing commas in expressions, one for declarations. |
This comment has been minimized.
This comment has been minimized.
@CyrusNajmabadi it seems like only one option would be needed here. I don't think StyleCop Analyzers ever had a request to split the single option in two, and even if it did it would be an extreme minority scenario. |
@CyrusNajmabadi If you're going to attempt this, please also update https://github.com/dotnet/docs/blob/master/docs/fundamentals/code-analysis/style-rules/formatting-rules.md to reflect this new feature for the .editorconfig. I requested this previously in: dotnet/docs#22120 |
Also, yes, I don't think we need two options. IMHO, the whole point of this rule is so that diffs are easier to read. |
For what it's worth, we'd use this in every repo for a cleaner history - arrived at this issue when looking for the functionality. |
@heaths were you able to make any progress on this? I'm starting on a new repo where I would like to have this option, and may be willing to implement it. |
Sorry, no. Other shiny things grabbed my attention. Have at it. 🙂 |
Would be super awesome if you could do this @eatdrinksleepcode ! ☕ |
@eatdrinksleepcode Did you by chance start this? We're thinking about potentially picking this up directly on our team, but wouldn't want to preempt you if you were making progress. |
@jasonmalinowski I have not made any progress on it. Other shiny things also grabbed my attention :) Please, please do add it. |
I've already a PR out, which I hope gets merged soon. |
@Youssef1313 Ah, not sure why we didn't assign this to you then! |
It would be lovely to see this land someday. As someone coming back to C# after much time away playing with other languages/frameworks, it's surprising that this isn't easy to enforce in a C# codebase (e.g. like it is with ESLint's |
@QuintinWillison we'd welcome a PR :-) |
@CyrusNajmabadi I wish I had enough free hours in my day to even imagine getting involved that deeply! 😆 ... but, in all seriousness, having noticed @Youssef1313 had already submitted one (#54859) then I was hoping my message might provide a nudge in the right direction, for it's not obvious (from a distance) as to what's blocking this other than time/availability of those already involved. Also, open source is lovely and all that. A "foundation" sounds great, too... but don't Microsoft ultimately earn dollar from this stuff, on numerous levels of aggregation? Feels like tools being left to languish to me by the corporation/foundation, not to mention the work of open source contributors like Youssef clearly not being valued enough by that corporate/foundation to be carried over the line and merged. It all feels somewhat under resourced and ultimately pretty slapdash to me. 🤷 |
Sure? But that doesn't mean we have teh resources to address the 8000 issues files against this repo that people want. Ultimately, we are a team with a set of time and resource constraints, like anything else. And we make decisions on where to allocate those resources. If that allocation doesn't align with the allocation you would prefer, open source provides an avenue for you to get involved and make things happen you find more important than what is on our current roster.
I don't know what the basis of this is. We've fixed literally thousands of bugs, and continue to drive new features, quality, scalability, performance and more that our ecosystem has been strongly asking for. We will not get to everything that everyone wants though. And expecting that will only make you disappointed. However unlike the closed source days, where if we didn't do something you were SOL, you now you have the option to get involved and get changes that are important to you.
I don't know what you mean. Youssef has the most contributions to roslyn accepted from an external contributor than anyone else. He's worked on the IDE, on the Compiler and on the Language itself. We value his contributions enormously, and we work with him in close partnership here and in our other communities (like Discord). But we also value when he has his own stuff to take care of, and we will happily work with him more if he has more time to give to the Roslyn project.
We'd welcome your help and contributions here. If you're not happy that we're not prioritizing though the set of items you think are more important to you, i cannot help you with that. :-/ |
Version Used: 15.5.5
Steps to Reproduce:
Expected Behavior:
I have an .editorconfig option to force developer to add comma after Second. It's useful, because it allows easy copy/paste and reordering member, and most important, when somebody else will add Third member, line with "Second" won't be changed and won't appear in comparsions.
Actual Behavior:
I have no way to force to add trailing comma.
P.S. Sorry for my English not being 100% sharp.
The text was updated successfully, but these errors were encountered: