patch to preserve carriage returns when using clang-format VS plugin

jpark37 . jpark37 at gmail.com
Thu Nov 28 03:56:41 PST 2013


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 <klimek at google.com> wrote:

> On Thu, Nov 28, 2013 at 12:32 PM, Alp Toker <alp at nuanti.com> 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 . <jpark37 at gmail.com <mailto:
>>> jpark37 at gmail.com>> 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 <djasper at google.com
>>>     <mailto:djasper at google.com>> 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 . <jpark37 at gmail.com
>>>         <mailto:jpark37 at gmail.com>> 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
>>>             cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>>             http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>> --
>> http://www.nuanti.com
>> the browser experts
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131128/aaf99d84/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ClangFormat.zip
Type: application/zip
Size: 710 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131128/aaf99d84/attachment.zip>


More information about the cfe-commits mailing list