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

Alp Toker alp at nuanti.com
Thu Nov 28 03:32:28 PST 2013


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 :-)

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




More information about the cfe-commits mailing list