Looks good - do you need me to submit it or do you have submit access? Cheers, /Manuel
On Thu, Nov 28, 2013 at 12:56 PM, jpark37 . <[email protected]> wrote: > 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. > > As Manuel said, the escaping cases aren't going to expand beyond what's > needed for newlines. > > > On Thu, Nov 28, 2013 at 6:53 AM, Manuel Klimek <[email protected]> wrote: > >> On Thu, Nov 28, 2013 at 12:32 PM, Alp Toker <[email protected]> wrote: >> >>> 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. >>> >>> clang-format deals primarily with whitespace, and that's ironically one >>> of the hardest things to preserve effectively in XML. >>> >>> 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. >>> >>> Alternatively, could expose a libFormat entry point, deploy as a DLL and >>> P/Invoke it directly from the VS extension. >>> >>> I can help out with either of these -- let's put an end to "XML" >>> implementations in clang :-) >>> >> >> 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. >> >> 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. >> >> Cheers, >> /Manuel >> >> >>> >>> Alp. >>> >>> >>> >>> >>> On 28/11/2013 10:36, Manuel Klimek wrote: >>> >>>> Index: tools/clang-format/ClangFormat.cpp >>>> =================================================================== >>>> --- tools/clang-format/ClangFormat.cpp (revision 195826) >>>> +++ tools/clang-format/ClangFormat.cpp (working copy) >>>> @@ -173,6 +173,27 @@ >>>> return false; >>>> } >>>> +static void outputReplacementXML(StringRef Text) { >>>> + const char *Data = Text.data(); >>>> >>>> There's usually no need to go back to raw char *'s when you have >>>> StringRef's (they kinda replace raw char *s). >>>> >>>> + size_t From = 0; >>>> >>>> Any reason to have different types for From and Index? >>>> >>>> + StringRef::size_type Index; >>>> + while ((Index = Text.find_first_of("\n\r", From)) != >>>> StringRef::npos) { >>>> + llvm::outs().write(Data + From, Index - From); >>>> >>>> llvm::outs() << Text.substr(From, Index - From); >>>> >>>> + switch (Data[Index]) { >>>> + case '\n': >>>> + llvm::outs() << " "; >>>> + break; >>>> + case '\r': >>>> + llvm::outs() << " "; >>>> + break; >>>> + default: >>>> + llvm::errs() << "error: unexpected character encountered\n"; >>>> >>>> As this would be a logic error, I'd use llvm_unreachable(...); >>>> >>>> + } >>>> + From = Index + 1; >>>> + } >>>> + llvm::outs().write(Data + From, Text.size() - From); >>>> >>>> llvm::outs() << Text.substr(From); >>>> >>>> +} >>>> + >>>> // Returns true on error. >>>> static bool format(StringRef FileName) { >>>> FileManager Files((FileSystemOptions())); >>>> @@ -205,8 +226,9 @@ >>>> I != E; ++I) { >>>> llvm::outs() << "<replacement " >>>> << "offset='" << I->getOffset() << "' " >>>> - << "length='" << I->getLength() << "'>" >>>> - << I->getReplacementText() << "</replacement>\n"; >>>> + << "length='" << I->getLength() << "'>"; >>>> + outputReplacementXML(I->getReplacementText()); >>>> + llvm::outs() << "</replacement>\n"; >>>> } >>>> llvm::outs() << "</replacements>\n"; >>>> } else { >>>> >>>> >>>> >>>> On Thu, Nov 28, 2013 at 12:42 AM, jpark37 . <[email protected] <mailto: >>>> [email protected]>> wrote: >>>> >>>> Oops, sorry; the attached patch is updated and retested. I ran >>>> clang-format, and it created more diffs than just my changes; >>>> those have been undone to keep the patch focused. I've also >>>> switched the cascading if to a switch statement. >>>> >>>> - James >>>> >>>> >>>> On Wed, Nov 27, 2013 at 5:24 PM, Daniel Jasper <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> I'd like Manuel to take a look, but in general, please format >>>> Clang/LLVM files with the correct style (i.e. "clang-format >>>> -style LLVM") :-). >>>> >>>> >>>> On Wed, Nov 27, 2013 at 11:28 AM, jpark37 . <[email protected] >>>> <mailto:[email protected]>> wrote: >>>> >>>> Hello there, >>>> >>>> I'm seeing newlines without carriage returns when using >>>> the clang-format plugin for Visual Studio. The issue seems >>>> to be that clang-format is not escaping newline characters >>>> when run with -output-replacements-xml, so the .NET XML >>>> stuff ends up collapsing \r\n down to \n. I've attached a >>>> patch that I've tested and appears to address the problem. >>>> >>>> - James >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] <mailto:[email protected]> >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> [email protected] >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>> >>> -- >>> http://www.nuanti.com >>> the browser experts >>> >>> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
