Page MenuHomePhabricator

Treat C# `using` as a control statement
ClosedPublic

Authored by jbcoe on Jan 3 2020, 5:36 AM.

Details

Summary

Unless SpaceBeforeParensOptions is set to SBPO_Never, a space will be put between using and ( in C# code.

Diff Detail

Event Timeline

jbcoe created this revision.Jan 3 2020, 5:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 3 2020, 5:36 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
jbcoe updated this revision to Diff 236035.Jan 3 2020, 5:40 AM
jbcoe added a reviewer: klimek.
This revision is now accepted and ready to land.Jan 4 2020, 2:08 AM
krasimir requested changes to this revision.Jan 11 2020, 2:24 AM
krasimir added inline comments.
clang/unittests/Format/FormatTestCSharp.cpp
265

Please also add a test for SBPO_NonEmptyParentheses.

267

Can this appear at the top-level? If not, just keep the other test case to avoid confusion.

This revision now requires changes to proceed.Jan 11 2020, 2:24 AM
jbcoe updated this revision to Diff 239574.Jan 22 2020, 6:38 AM

Handle using case where SpaceBeforeParensOptions is set to SBPO_NonEmptyParentheses

jbcoe marked 2 inline comments as done.Jan 22 2020, 6:39 AM
jbcoe added inline comments.
clang/unittests/Format/FormatTestCSharp.cpp
267

I'm following the example set by by test cases above. I can remove (all) if you prefer and agree it does not test anything new.

krasimir accepted this revision.Jan 22 2020, 8:27 AM
krasimir added inline comments.
clang/unittests/Format/FormatTestCSharp.cpp
267

I didn't notice the old example. Yes, please remove all of them.

This revision is now accepted and ready to land.Jan 22 2020, 8:27 AM
jbcoe updated this revision to Diff 239629.Jan 22 2020, 9:37 AM

Removed test snippets that did not exercise new code.

jbcoe marked an inline comment as done.Jan 22 2020, 9:37 AM
MyDeveloperDay requested changes to this revision.EditedJan 22 2020, 12:01 PM

Removed test snippets that did not exercise new code.

Actually i would prefer we didn't remove those tests, because we added them because previously someone called out that it had that effect when at the top level

because clang-format can work on a snippet of code it may be that you don't see the outer scope, I see those tests doing no harm and they assert the behaviour that was previously seen to be different for both in scope and toplevel

This revision now requires changes to proceed.Jan 22 2020, 12:01 PM
jbcoe updated this revision to Diff 239836.Jan 23 2020, 2:55 AM

Extended tests and added some comments explaining why seeming duplicated tests are useful.

krasimir requested changes to this revision.Jan 23 2020, 3:20 AM

I looked up the C# reference and all the examples are top-level, so having top-level test cases here would be useful, as people would expect copy-pasting an example snippet to just work.
Sorry about asking you to go back-and-forth.

This revision now requires changes to proceed.Jan 23 2020, 3:20 AM
krasimir accepted this revision.Jan 23 2020, 3:21 AM

I'll submit this for you.

This revision was not accepted when it landed; it landed in state Needs Revision.Jan 23 2020, 4:23 AM
This revision was automatically updated to reflect the committed changes.