[PATCH] D78642: [clang-format] Handle C# property accessors when parsing lines

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Apr 24 01:34:27 PDT 2020


MyDeveloperDay added inline comments.


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1344
           addUnwrappedLine();
         FormatTok->Type = TT_FunctionLBrace;
         parseBlock(/*MustBeDeclaration=*/false);
----------------
jbcoe wrote:
> MyDeveloperDay wrote:
> > previously set and get would break based on the setting of AfterFunction correct? now I assume it doesn't?
> 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
Sounds good


================
Comment at: clang/unittests/Format/FormatTestCSharp.cpp:249
+               "public string Host { set; get; }");
 
   verifyFormat("[TestMethod(\"start\", HelpText = \"Starts the server "
----------------
jbcoe wrote:
> MyDeveloperDay wrote:
> > 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?
> 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.
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78642/new/

https://reviews.llvm.org/D78642





More information about the cfe-commits mailing list