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

[Manuel Klimek]

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

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/e59cac30/attachment.html>