<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Dec 17, 2012, at 2:52 PM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr">On Mon, Dec 17, 2012 at 11:04 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank">dgregor@apple.com</a>></span> wrote:<br>
<div class="gmail_extra"><div class="gmail_quote"><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">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><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></div></blockquote><div><br></div><div style="">As far as I'm aware the RewriterTestContext has nothing to do with formatting - it has all to do with combining all the dependencies of the *Rewriter* into a single instantiable entity. To trigger formatting, you need a Lexer, a SourceManager and a configuration - that's all.</div>
<div style=""><br></div><div style="">So, to summarize what I said in an earlier mail, I would propose the following (if we decide we really want something along those lines):</div><div style="">- come up with an interface that clients like libclang would need; from what I can see that would be putting together all the dependencies of the Rewriter into an easy-to-instantiate bundle and having a covenience method to create an in-memory file on the source manager (from what I saw the proposed change does not use all the temp file magic, which is both the most brittle and what I don't expect anything bu the test to use)</div>
<div style="">- put that into a decently named class in Rewrite/; perhaps RewriterDependencies, or RewriterContext, or VirtualRewriterContext (to underline the use for virtual files)</div></div></div></div></div></blockquote><div><br></div>I think we're mostly in agreement here. VirtualRewriterContext in Rewrite would be fine by me.</div><div><br></div><div><blockquote type="cite"><div style="font-family: arial, helvetica, sans-serif; font-size: 10pt"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div style="">But to take a step back (and maybe I missed something) - doesn't libclang already have a way to instantiate everything needed for Rewriting a bunch of files?</div>
</div></div></div></div></blockquote><div><br></div><div>We're not rewriting files, we're rewriting text produced by calling Decl::print(). Since we'll be doing this a lot within libclang, we need to keep the context object around so we're not rebuilding the SourceManager, Lexer, etc.</div><br><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug</div><br></body></html>