Page MenuHomePhabricator

[clang-format] Handle C# property accessors when parsing lines
ClosedPublic

Authored by jbcoe on Apr 22 2020, 8:03 AM.

Details

Summary

Improve C# { get; set; } = default; formatting by handling it in the UnwrappedLineParser rather than trying to merge lines later.

Remove old logic to merge lines.

Update tests as formatting output has changed (as intended).

Diff Detail

Event Timeline

jbcoe created this revision.Apr 22 2020, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2020, 8:03 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jbcoe updated this revision to Diff 259409.Apr 22 2020, 2:49 PM

Format patch

krasimir accepted this revision.Apr 23 2020, 3:13 AM

Thank you!

This revision is now accepted and ready to land.Apr 23 2020, 3:13 AM
This revision was automatically updated to reflect the committed changes.
MyDeveloperDay added inline comments.Apr 23 2020, 6:30 AM
clang/unittests/Format/FormatTestCSharp.cpp
249

is this just a personal choice? or based on some rule that it shouldn't break?

I don't like us changing tests unless we understand otherwise we just keep flip-flopping the style?

MyDeveloperDay added inline comments.Apr 23 2020, 6:32 AM
clang/lib/Format/UnwrappedLineParser.cpp
1344

previously set and get would break based on the setting of AfterFunction correct? now I assume it doesn't?

jbcoe marked 2 inline comments as done.Apr 23 2020, 10:00 AM
jbcoe added inline comments.
clang/lib/Format/UnwrappedLineParser.cpp
1344

That's right. There's a bunch more work needed here and current/previous behaviour is undertested and incorrect.

I'm focusing on this for the next few days so should get everything working well and configurable as we'd like.

MS examples are not very consistent so choice seems like the way forward: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/language-specification/classes#properties

clang/unittests/Format/FormatTestCSharp.cpp
249

Agreed. This was oversight and merits discussion. I'll make this configurable in a follow-up patch.

Thanks for taking the time to review/comment.

MyDeveloperDay added inline comments.Apr 24 2020, 1:16 AM
clang/lib/Format/UnwrappedLineParser.cpp
1344

Sounds good

clang/unittests/Format/FormatTestCSharp.cpp
249

I the original C# work I mentioned we'd want to add specific styling for things like this, I feel this is going the the right direction thank you.