krasimir created this revision. Herald added a subscriber: klimek. This patch detects the leading '<' in likely xml files and stops formatting in that case. A recent use of a Qt xml file with a .ts extension triggered this: http://doc.qt.io/qt-4.8/linguist-ts-file-format.html
https://reviews.llvm.org/D37136 Files: lib/Format/Format.cpp unittests/Format/FormatTest.cpp unittests/Format/SortIncludesTest.cpp Index: unittests/Format/SortIncludesTest.cpp =================================================================== --- unittests/Format/SortIncludesTest.cpp +++ unittests/Format/SortIncludesTest.cpp @@ -398,6 +398,17 @@ EXPECT_EQ(26u, Ranges[0].getLength()); } +TEST_F(SortIncludesTest, DoNotSortLikelyXml) { + EXPECT_EQ("<!--;\n" + "#include <b>\n" + "#include <a>\n" + "-->", + sort("<!--;\n" + "#include <b>\n" + "#include <a>\n" + "-->")); +} + } // end namespace } // end namespace format } // end namespace clang Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11217,6 +11217,13 @@ EXPECT_EQ("auto c = u8'a';", format("auto c = u8'a';")); } +TEST_F(FormatTest, DoNotFormatLikelyXml) { + EXPECT_EQ("<!-- ;> -->", + format("<!-- ;> -->", getGoogleStyle())); + EXPECT_EQ(" <!-- >; -->", + format(" <!-- >; -->", getGoogleStyle())); +} + } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -1539,14 +1539,19 @@ return Code.size() > 188 && Code[0] == 0x47 && Code[188] == 0x47; } +bool likelyXml(StringRef Code) { + return Code.ltrim().startswith("<"); +} + tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, ArrayRef<tooling::Range> Ranges, StringRef FileName, unsigned *Cursor) { tooling::Replacements Replaces; if (!Style.SortIncludes) return Replaces; - if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript && - isMpegTS(Code)) + if (likelyXml(Code) || + (Style.Language == FormatStyle::LanguageKind::LK_JavaScript && + isMpegTS(Code))) return Replaces; if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript) return sortJavaScriptImports(Style, Code, Ranges, FileName); @@ -1894,7 +1899,8 @@ FormatStyle Expanded = expandPresets(Style); if (Expanded.DisableFormat) return tooling::Replacements(); - if (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code)) + if (likelyXml(Code) || + (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code))) return tooling::Replacements(); typedef std::function<tooling::Replacements(const Environment &)>
Index: unittests/Format/SortIncludesTest.cpp =================================================================== --- unittests/Format/SortIncludesTest.cpp +++ unittests/Format/SortIncludesTest.cpp @@ -398,6 +398,17 @@ EXPECT_EQ(26u, Ranges[0].getLength()); } +TEST_F(SortIncludesTest, DoNotSortLikelyXml) { + EXPECT_EQ("<!--;\n" + "#include <b>\n" + "#include <a>\n" + "-->", + sort("<!--;\n" + "#include <b>\n" + "#include <a>\n" + "-->")); +} + } // end namespace } // end namespace format } // end namespace clang Index: unittests/Format/FormatTest.cpp =================================================================== --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -11217,6 +11217,13 @@ EXPECT_EQ("auto c = u8'a';", format("auto c = u8'a';")); } +TEST_F(FormatTest, DoNotFormatLikelyXml) { + EXPECT_EQ("<!-- ;> -->", + format("<!-- ;> -->", getGoogleStyle())); + EXPECT_EQ(" <!-- >; -->", + format(" <!-- >; -->", getGoogleStyle())); +} + } // end namespace } // end namespace format } // end namespace clang Index: lib/Format/Format.cpp =================================================================== --- lib/Format/Format.cpp +++ lib/Format/Format.cpp @@ -1539,14 +1539,19 @@ return Code.size() > 188 && Code[0] == 0x47 && Code[188] == 0x47; } +bool likelyXml(StringRef Code) { + return Code.ltrim().startswith("<"); +} + tooling::Replacements sortIncludes(const FormatStyle &Style, StringRef Code, ArrayRef<tooling::Range> Ranges, StringRef FileName, unsigned *Cursor) { tooling::Replacements Replaces; if (!Style.SortIncludes) return Replaces; - if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript && - isMpegTS(Code)) + if (likelyXml(Code) || + (Style.Language == FormatStyle::LanguageKind::LK_JavaScript && + isMpegTS(Code))) return Replaces; if (Style.Language == FormatStyle::LanguageKind::LK_JavaScript) return sortJavaScriptImports(Style, Code, Ranges, FileName); @@ -1894,7 +1899,8 @@ FormatStyle Expanded = expandPresets(Style); if (Expanded.DisableFormat) return tooling::Replacements(); - if (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code)) + if (likelyXml(Code) || + (Expanded.Language == FormatStyle::LK_JavaScript && isMpegTS(Code))) return tooling::Replacements(); typedef std::function<tooling::Replacements(const Environment &)>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits