<div dir="ltr"><div>I do need this submitted for me. Thanks Manuel.<br><br></div>- James<br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 28, 2013 at 7:45 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Looks good - do you need me to submit it or do you have submit access?<div><br></div><div>Cheers,</div><div>
/Manuel</div></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Nov 28, 2013 at 12:56 PM, jpark37 . <span dir="ltr"><<a href="mailto:jpark37@gmail.com" target="_blank">jpark37@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>I had different size types to try to match the function signature
of find_first_of in StringRef.cpp, which is actually different from the
signature in StringRef.h. (This inconsistency should probably be fixed
at some point.) I've fixed up the patch, retested, and attached.<br><br></div>As Manuel said, the escaping cases aren't going to expand beyond what's needed for newlines.<br></div><div><div>
<div class="gmail_extra"><br>
<br><div class="gmail_quote">On Thu, Nov 28, 2013 at 6:53 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>On Thu, Nov 28, 2013 at 12:32 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br>
</div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
So, I think XML is just the wrong format to use here. There have been various XML implementations in LLVM and they all started like this, then went downhill as the need for more entity escaping rules came up until finally getting removed.<br>
<br>
clang-format deals primarily with whitespace, and that's ironically one of the hardest things to preserve effectively in XML.<br>
<br>
How about getting clang-format to generate an edit script? They're dead simple to generate in C++, easy to parse in C#, and can be applied with standard tools like ed or diffutils for testing. No escaping needed.<br>
<br>
Alternatively, could expose a libFormat entry point, deploy as a DLL and P/Invoke it directly from the VS extension.<br>
<br>
I can help out with either of these -- let's put an end to "XML" implementations in clang :-)<br></blockquote><div><br></div></div><div>I would also have preferred to not to produce XML in clang-format. Note that it was not done for VS, but for the Eclipse integration.</div>
<div><br></div><div>I read up on the XML spec, and whitespace doesn't seem hard to preserve here - an XML implementation must provide all whitespace as-is; the one problem is that the XML spec actually enforces normalization of newlines to \n before parsing, which makes it look like this is the only XML specific thing we need.</div>
<div><br></div><div>Cheers,</div><div>/Manuel</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Alp.<div><div><br>
<br>
<br>
<br>
On 28/11/2013 10:36, Manuel Klimek wrote:<br>
</div></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
Index: tools/clang-format/<u></u>ClangFormat.cpp<br>
==============================<u></u>==============================<u></u>=======<br>
--- tools/clang-format/<u></u>ClangFormat.cpp (revision 195826)<br>
+++ tools/clang-format/<u></u>ClangFormat.cpp (working copy)<br>
@@ -173,6 +173,27 @@<br>
return false;<br>
}<br>
+static void outputReplacementXML(StringRef Text) {<br>
+ const char *Data = Text.data();<br>
<br>
There's usually no need to go back to raw char *'s when you have StringRef's (they kinda replace raw char *s).<br>
<br>
+ size_t From = 0;<br>
<br>
Any reason to have different types for From and Index?<br>
<br>
+ StringRef::size_type Index;<br>
+ while ((Index = Text.find_first_of("\n\r", From)) != StringRef::npos) {<br>
+ llvm::outs().write(Data + From, Index - From);<br>
<br>
llvm::outs() << Text.substr(From, Index - From);<br>
<br>
+ switch (Data[Index]) {<br>
+ case '\n':<br>
+ llvm::outs() << " ";<br>
+ break;<br>
+ case '\r':<br>
+ llvm::outs() << " ";<br>
+ break;<br>
+ default:<br>
+ llvm::errs() << "error: unexpected character encountered\n";<br>
<br>
As this would be a logic error, I'd use llvm_unreachable(...);<br>
<br>
+ }<br>
+ From = Index + 1;<br>
+ }<br>
+ llvm::outs().write(Data + From, Text.size() - From);<br>
<br>
llvm::outs() << Text.substr(From);<br>
<br>
+}<br>
+<br>
// Returns true on error.<br>
static bool format(StringRef FileName) {<br>
FileManager Files((FileSystemOptions()));<br>
@@ -205,8 +226,9 @@<br>
I != E; ++I) {<br>
llvm::outs() << "<replacement "<br>
<< "offset='" << I->getOffset() << "' "<br>
- << "length='" << I->getLength() << "'>"<br>
- << I->getReplacementText() << "</replacement>\n";<br>
+ << "length='" << I->getLength() << "'>";<br>
+ outputReplacementXML(I-><u></u>getReplacementText());<br>
+ llvm::outs() << "</replacement>\n";<br>
}<br>
llvm::outs() << "</replacements>\n";<br>
} else {<br>
<br>
<br>
<br></div></div><div>
On Thu, Nov 28, 2013 at 12:42 AM, jpark37 . <<a href="mailto:jpark37@gmail.com" target="_blank">jpark37@gmail.com</a> <mailto:<a href="mailto:jpark37@gmail.com" target="_blank">jpark37@gmail.com</a>>> wrote:<br>
<br>
Oops, sorry; the attached patch is updated and retested. I ran<br>
clang-format, and it created more diffs than just my changes;<br>
those have been undone to keep the patch focused. I've also<br>
switched the cascading if to a switch statement.<br>
<br>
- James<br>
<br>
<br>
On Wed, Nov 27, 2013 at 5:24 PM, Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a><br></div><div>
<mailto:<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>>> wrote:<br>
<br>
I'd like Manuel to take a look, but in general, please format<br>
Clang/LLVM files with the correct style (i.e. "clang-format<br>
-style LLVM") :-).<br>
<br>
<br>
On Wed, Nov 27, 2013 at 11:28 AM, jpark37 . <<a href="mailto:jpark37@gmail.com" target="_blank">jpark37@gmail.com</a><br></div><div>
<mailto:<a href="mailto:jpark37@gmail.com" target="_blank">jpark37@gmail.com</a>>> wrote:<br>
<br>
Hello there,<br>
<br>
I'm seeing newlines without carriage returns when using<br>
the clang-format plugin for Visual Studio. The issue seems<br>
to be that clang-format is not escaping newline characters<br>
when run with -output-replacements-xml, so the .NET XML<br>
stuff ends up collapsing \r\n down to \n. I've attached a<br>
patch that I've tested and appears to address the problem.<br>
<br>
- James<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br></div>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.<u></u>edu</a>><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><div><br>
<br>
<br>
<br>
<br>
<br>
<br>
______________________________<u></u>_________________<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/<u></u>mailman/listinfo/cfe-commits</a><br>
</div></blockquote><span><font color="#888888">
<br>
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div></div>