patch to preserve carriage returns when using clang-format VS plugin
Manuel Klimek
klimek at google.com
Tue Dec 3 01:50:35 PST 2013
Thx, committed as r196265
On Thu, Nov 28, 2013 at 7:47 PM, James Park <jpark37 at gmail.com> wrote:
> I do need this submitted for me. Thanks Manuel.
>
> - James
>
>
>
> On Thu, Nov 28, 2013 at 7:45 AM, Manuel Klimek <klimek at google.com> wrote:
>
>> 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 . <jpark37 at gmail.com> 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 <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/20131203/74b098c0/attachment.html>
More information about the cfe-commits
mailing list