[cfe-commits] clang formatter patch to move and rename RewriterTestContext.h

[Daniel Jasper]

Hi Fariborz,

I am taking a look at your patch, but please bear with me for another day
or two. I am not yet convinced that mingling C++ and ObjC formatting that
closely is a wise decision. I think we might need something that clearly
separates the different modes. And you touch a lot of code that I intended
to refactor into something halfway sane this week. I will take your patch
into consideration when doing so, but it might need some changes afterwards.

As for RewriterTestContext, please try to figure out whether that is really
something you want to reuse in libclang. If so, I am happy to move it into
include/clang/Tooling (or you could do that yourself). But I suspect that
we want something different there, possibly a completely different
interface.

Cheers,
Daniel


On Mon, Dec 17, 2012 at 6:47 PM, jahanian <fjahanian at apple.com> wrote:

>
> On Dec 17, 2012, at 9:36 AM, Manuel Klimek <klimek at google.com> wrote:
>
> I don't think it's the right design to put something that is generally
> thought for testing into a public header and use it from libclang.
> If we want something similar to RewriterTestContext, it should:
> a) live next to the Rewriter in clang - as it provides everything needed
> to rewrite files
> b) leave out all functionality that's not needed outside of tests
> c) be proposed and reviewed in an extra patch, which I'd assume would need
> approval from Doug - it's a non-trivial addition to the public api
>
>
> I agree with all these. I wanted to move forward with ObjC formatting and
> it was a temporary refactoring allowing me to move forward. Form and
> placement
> of the public header rests with Daniel. Also my patch includes complete
> formatting of ObjC declarations which I hope will be useful.
>
> - Fariborz
>
>
> Now, I'm all for having some nicer "just bundle me something up" APIs so a
> lot of Clang's infrastructure like the Rewriter gets simpler to use, but
> I'd rather see it introduced in a deliberate way.
>
> Thanks!
> /Manuel
>
>
>
> On Mon, Dec 17, 2012 at 6:13 PM, jahanian <fjahanian at apple.com> wrote:
>
>> Hi Daniel,
>>
>> Are you going to provide the new public header in clang/Tooling this
>> week? I have gone ahead and made the necessary changes for formatting ObjC
>> declarations and
>> and provide the tests in the attached patch which enforces our convention
>> for formatting them.  Here is my complete patch (including the plumbing for
>> declaration tags which you may ignore).  How do you want to proceed?
>>
>> - Thanks, Fariborz
>>
>>
>>
>> On Dec 16, 2012, at 10:21 PM, Daniel Jasper <djasper at google.com> wrote:
>>
>> I think the RewriterTestContext should remain in Tooling as it has
>> nothing to do with formatting. I guess, we could make it a public header
>> there (clang/Tooling). And it should remain being called
>> RewriterTestContext.
>>
>> The ObjC change looks fine as a temporary fix. Especially the part in
>> Format.cpp will undergo a rewrite anyway, hopefully this week. I will look
>> into what else needs to change to support ObjC. At the very least, we
>> probably shouldn't set CPlusPlus LangOpts for the Lexer..
>>
>>
>>
>> On Wed, Dec 12, 2012 at 8:50 PM, Sean Silva <silvas at purdue.edu> wrote:
>>
>>> On Wed, Dec 12, 2012 at 1:00 PM, jahanian <fjahanian at apple.com> wrote:
>>> > ter. This patch is an attempt to have a header in Format directory
>>> > that others can use. Any suggestion is welcome.
>>>
>>> +djasper
>>>
>>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121217/89382006/attachment.html>