<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><div class="gmail_default" style><br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Dec 17, 2012 at 11:04 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank" class="cremed">dgregor@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:55 AM, Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>> wrote:</div>
<br></div><div class="im"><blockquote type="cite"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div dir="ltr">Hi Fariborz,<div><br></div><div>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></div></blockquote><div><br></div></div>If we're going to distinguish the modes, we need to do so syntactically. Objective-C++ is a very popular dialect; we can't simply rely on extrinsic knowledge of whether we're dealing with Objective-C or C++ to make these decisions.</div>
</div></blockquote><div><br></div><div style>I am not sure I fully understand this. We are currently relying on a Lexer which in turn requires LangOptions to create the correct tokens. Same for the identifier table we use to split raw_identifiers into identifiers and keywords. Are you saying the formatter should not capitalize on this knowledge of the input token stream?</div>
<div style> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div class="im"><br><blockquote type="cite"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt">
<div dir="ltr"><div>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></div></blockquote><div><br></div></div><div>It is. At least, we need a persistent context that we can use to apply the code formatter to code snippets produced within libclang. We could pull out just the parts of RewriterTestContext that we need and give it a new name (SimpleFormatContext, for example).</div>
<div><br></div><div><span style="white-space:pre-wrap"> </span>- Doug</div><div><div class="h5"><br><blockquote type="cite"><div style="font-family:arial,helvetica,sans-serif;font-size:10pt"><div class="gmail_extra"><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" class="cremed">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><div>On Dec 17, 2012, at 9:36 AM, Manuel Klimek <<a href="mailto:klimek@google.com" target="_blank" class="cremed">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><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" class="cremed">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" class="cremed">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" class="cremed">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" class="cremed">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" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">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>
_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="cremed">cfe-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></blockquote></div><br></div></div></div>