<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr">Hi Fariborz,<div><br></div><div style>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.</div>
<div style><br></div><div style>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.</div>
<div style><br></div><div style>Cheers,<br>Daniel</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Dec 17, 2012 at 6:47 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div class="im"><div>On Dec 17, 2012, at 9:36 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div dir="ltr">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.<div>
If we want something similar to RewriterTestContext, it should:</div><div>a) live next to the Rewriter in clang - as it provides everything needed to rewrite files</div><div>b) leave out all functionality that's not needed outside of tests</div>
<div>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</div></div></div></blockquote><div><br></div></div>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</div>
<div>of the public header rests with Daniel. Also my patch includes complete formatting of ObjC declarations which I hope will be useful.</div><div><br></div><div>- Fariborz</div><div class="im"><div><br></div><div><blockquote type="cite">
<div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div dir="ltr"><div><br></div><div>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.</div>
<div><br></div><div>Thanks!</div><div>/Manuel</div><div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Dec 17, 2012 at 6:13 PM, jahanian <span dir="ltr"><<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">Hi Daniel,<div><br></div><div>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</div>
<div>and provide the tests in the attached patch which enforces our convention for formatting them. Here is my complete patch (including the plumbing for </div><div>declaration tags which you may ignore). How do you want to proceed?</div>
<div><br></div><div>- Thanks, Fariborz</div><div><br></div><div><span style="white-space:pre-wrap"> </span></div></div>
<br><div style="word-wrap:break-word"><div></div><div><br><div><div>On Dec 16, 2012, at 10:21 PM, Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>> wrote:</div><br><blockquote type="cite">
<div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div dir="ltr">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.<div>
<br></div><div>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..</div>
<div><br></div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Dec 12, 2012 at 8:50 PM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>On Wed, Dec 12, 2012 at 1:00 PM, jahanian <<a href="mailto:fjahanian@apple.com" target="_blank">fjahanian@apple.com</a>> wrote:<br>
> ter. This patch is an attempt to have a header in Format directory<br>
> that others can use. Any suggestion is welcome.<br>
<br>
</div>+djasper<br>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div></div>
</blockquote></div><br></div></div>