Page MenuHomePhabricator

[clang-format] Wrap lines for C# property accessors
ClosedPublic

Authored by jbcoe on Feb 21 2020, 6:33 PM.

Details

Summary

Ensure that auto-implemented properties { get; private set } are wrapped on to one line for C# code.

Diff Detail

Event Timeline

jbcoe created this revision.Feb 21 2020, 6:33 PM
jbcoe added a project: Restricted Project.
Eugene.Zelenko set the repository for this revision to rG LLVM Github Monorepo.
Eugene.Zelenko added a subscriber: cfe-commits.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2020, 9:58 PM
jbcoe updated this revision to Diff 246070.Feb 22 2020, 2:53 AM

Code and test to avoid wrapping accessors with expression body definitions.

MyDeveloperDay added inline comments.Feb 23 2020, 6:38 AM
clang/lib/Format/UnwrappedLineFormatter.cpp
443

I think we need to consider internal

456

and here

jbcoe updated this revision to Diff 246170.Feb 24 2020, 2:56 AM

Handle C# access modifier internal .

Fix typo in test for expression-bodied get/set methods.

MyDeveloperDay accepted this revision.Feb 24 2020, 4:00 AM

This LGTM, thank you for the patch

This revision is now accepted and ready to land.Feb 24 2020, 4:00 AM
jbcoe retitled this revision from Wrap lines for C# property accessors to [clang-format] Wrap lines for C# property accessors.Feb 24 2020, 6:09 AM
jbcoe planned changes to this revision.Feb 24 2020, 7:17 AM
jbcoe updated this revision to Diff 246235.Feb 24 2020, 9:34 AM

Simplify logic for merging property accessors

This revision is now accepted and ready to land.Feb 24 2020, 9:34 AM
jbcoe marked 2 inline comments as done.Feb 24 2020, 9:34 AM
jbcoe edited the summary of this revision. (Show Details)Feb 24 2020, 9:55 AM
krasimir accepted this revision.Feb 25 2020, 5:12 AM

This is pretty cool!

MyDeveloperDay accepted this revision.Feb 25 2020, 5:40 AM

LGTM

clang/unittests/Format/FormatTestCSharp.cpp
246

Nit: I feel like this layout should really be an option, (maybe for the future).

When we originally did the C# work, we did say we might want in the future to add options that might be specific like laying out the accessors

I think VS has support for formatting them as either

public int Goo { set; get; }

and

public int Goo
{
     set; get;
}

and the following is equally valid in my eyes

public int Goo
{
     set; 
     get;
}

as well as what is being proposed here of

public int Goo
{   set; get; }

I'm not completely sure how much the other options are controlling this at present and how much is not in the control of your merging

and how does that change when there is actually code in the setter and getter?

jbcoe marked an inline comment as done.Feb 25 2020, 5:43 AM
jbcoe added inline comments.
clang/unittests/Format/FormatTestCSharp.cpp
246

Agreed that this should be configured by an option in a future revision.

Closed by commit rG2bd6974aaa66: [clang-format] Wrap lines for C# property accessors (authored by Jonathan Coe <jbcoe@google.com>). · Explain WhyFeb 25 2020, 6:29 AM
This revision was automatically updated to reflect the committed changes.