2841: Resolve StringUtils header (TrenchBroom#2863)

* 2841: refactor and split up string utils * 2841: make KDL lib static, allows to hide STL includes from string utilities * 2841: resolving StringUtils wip * 2841: remove StringUtils header * 2841: fix build errors # Conflicts: # common/src/IO/Path.h * 2841: fix more build errors * 2841: enable implicit conversion warnings for clang and gcc, fix a few build problems * 2841: add missing include * 2841: fix implicit conversion warnings from GCC # Conflicts: # common/test/src/Model/BrushTest.cpp * 2841: make kdl header-only again * 2841: add missing tests for kdl * 2841: revert change to str_is_numeric
s3thra · Dec 8, 2019 · aae86ad · aae86ad
1 parent b5aa802
commit aae86ad
Show file tree

Hide file tree

Showing 145 changed files with 2,096 additions and 1,620 deletions.
diff --git a/cmake/Utils.cmake b/cmake/Utils.cmake
@@ -89,7 +89,7 @@ endmacro(SET_XCODE_ATTRIBUTES)
 
 macro(set_compiler_config TARGET)
     if(COMPILER_IS_CLANG)
-        target_compile_options(${TARGET} PRIVATE -Wall -Wextra -pedantic)
+        target_compile_options(${TARGET} PRIVATE -Wall -Wextra -Wconversion -pedantic)
         target_compile_options(${TARGET} PRIVATE -Wno-global-constructors -Wno-exit-time-destructors -Wno-padded -Wno-format-nonliteral -Wno-used-but-marked-unused)
 
         # disable C++98 compatibility warnings
@@ -105,7 +105,7 @@ macro(set_compiler_config TARGET)
         # FIXME: Suppress warnings in moc generated files:
         target_compile_options(${TARGET} PRIVATE -Wno-redundant-parens)
     elseif(COMPILER_IS_GNU)
-        target_compile_options(${TARGET} PRIVATE -Wall -Wextra -pedantic)
+        target_compile_options(${TARGET} PRIVATE -Wall -Wextra -Wconversion -pedantic)
         target_compile_options(${TARGET} PRIVATE "$<$<CONFIG:RELEASE>:-O3>")
 
         # FIXME: enable -Wcpp once we found a workaround for glew / QOpenGLWindow problem, see RenderView.h

diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt
@@ -499,7 +499,6 @@ set(COMMON_SOURCE
         ${COMMON_SOURCE_DIR}/Preference.cpp
         ${COMMON_SOURCE_DIR}/Preferences.cpp
         ${COMMON_SOURCE_DIR}/Reference.cpp
-        ${COMMON_SOURCE_DIR}/StringUtils.cpp
         ${COMMON_SOURCE_DIR}/TemporarilySetAny.cpp
         ${COMMON_SOURCE_DIR}/TrenchBroomApp.cpp
         ${COMMON_SOURCE_DIR}/TrenchBroomStackWalker.cpp
@@ -1072,14 +1071,13 @@ set(COMMON_HEADER
         ${COMMON_SOURCE_DIR}/StringSet.h
         ${COMMON_SOURCE_DIR}/StringStream.h
         ${COMMON_SOURCE_DIR}/StringType.h
-        ${COMMON_SOURCE_DIR}/StringUtils.h
         ${COMMON_SOURCE_DIR}/TemporarilySetAny.h
         ${COMMON_SOURCE_DIR}/TrenchBroomApp.h
         ${COMMON_SOURCE_DIR}/TrenchBroom.h
         ${COMMON_SOURCE_DIR}/TrenchBroomStackWalker.h
 )
 
-add_library(common OBJECT ${COMMON_SOURCE} ${COMMON_HEADER})
+add_library(common OBJECT ${COMMON_SOURCE})
 set_target_properties(common PROPERTIES AUTOMOC TRUE)
 target_compile_features(common PRIVATE cxx_std_17)
 target_include_directories(common PUBLIC ${COMMON_SOURCE_DIR})

diff --git a/common/benchmark/src/IO/TestParserStatus.cpp b/common/benchmark/src/IO/TestParserStatus.cpp
@@ -19,6 +19,7 @@
 
 #include "TestParserStatus.h"
 
+#include <string>
 
 namespace TrenchBroom {
     namespace IO {
@@ -35,7 +36,7 @@ namespace TrenchBroom {
 
         void TestParserStatus::doProgress(const double) {}
 
-        void TestParserStatus::doLog(const Logger::LogLevel level, const String& /* str */) {
+        void TestParserStatus::doLog(const Logger::LogLevel level, const std::string& /* str */) {
             m_statusCounts[level]++; // unknown map values are value constructed, which initializes to 0 for size_t
         }
     }

diff --git a/common/benchmark/src/Renderer/BrushRendererBenchmark.cpp b/common/benchmark/src/Renderer/BrushRendererBenchmark.cpp
@@ -47,8 +47,7 @@ namespace TrenchBroom {
             // make textures
             std::vector<Assets::Texture*> textures;
             for (size_t i = 0; i < NumTextures; ++i) {
-                const String textureName = "texture " + std::to_string(i);
-
+                const auto textureName = "texture " + std::to_string(i);
                 textures.push_back(new Assets::Texture(textureName, 64, 64));
             }
 

diff --git a/common/src/Assets/ColorRange.cpp b/common/src/Assets/ColorRange.cpp
@@ -19,14 +19,14 @@
 
 #include "ColorRange.h"
 
-#include "StringUtils.h"
+#include <kdl/string_utils.h>
 
 namespace TrenchBroom {
     namespace Assets {
         ColorRange::Type detectColorRange(const StringList& components);
 
         ColorRange::Type detectColorRange(const String& str) {
-            return detectColorRange(StringUtils::splitAndTrim(str, " "));
+            return detectColorRange(kdl::str_split(str, " "));
         }
 
         ColorRange::Type detectColorRange(const StringList& components) {

diff --git a/common/src/Assets/EntityDefinition.cpp b/common/src/Assets/EntityDefinition.cpp
@@ -20,7 +20,8 @@
 #include "EntityDefinition.h"
 
 #include "Assets/AttributeDefinition.h"
-#include "StringUtils.h"
+
+#include <kdl/string_compare.h>
 
 #include <algorithm>
 #include <cassert>
@@ -142,7 +143,7 @@ namespace TrenchBroom {
                 });
             else
                 std::sort(std::begin(result), std::end(result), [] (const EntityDefinition* lhs, const EntityDefinition* rhs) {
-                    const int strCmp = StringUtils::caseInsensitiveCompare(lhs->name(), rhs->name());
+                    const int strCmp = kdl::ci::compare(lhs->name(), rhs->name());
                     if (strCmp == 0) {
                         return lhs->usageCount() > rhs->usageCount();
                     } else {

diff --git a/common/src/Assets/EntityDefinitionFileSpec.cpp b/common/src/Assets/EntityDefinitionFileSpec.cpp
@@ -19,7 +19,8 @@
 
 #include "EntityDefinitionFileSpec.h"
 
-#include "StringUtils.h"
+
+#include <kdl/string_compare.h>
 
 #include <cassert>
 
@@ -30,12 +31,12 @@ namespace TrenchBroom {
         m_path("") {}
 
         EntityDefinitionFileSpec EntityDefinitionFileSpec::parse(const String& str) {
-            if (StringUtils::isPrefix(str, "external:")) {
+            if (kdl::cs::is_prefix(str, "external:")) {
                 const IO::Path path(str.substr(9));
                 return EntityDefinitionFileSpec::external(path);
             }
 
-            if (StringUtils::isPrefix(str, "builtin:")) {
+            if (kdl::cs::is_prefix(str, "builtin:")) {
                 const IO::Path path(str.substr(8));
                 return EntityDefinitionFileSpec::builtin(path);
             }

diff --git a/common/src/Assets/EntityDefinitionGroup.cpp b/common/src/Assets/EntityDefinitionGroup.cpp
@@ -19,7 +19,7 @@
 
 #include "EntityDefinitionGroup.h"
 
-#include "StringUtils.h"
+#include <kdl/string_format.h>
 
 namespace TrenchBroom {
     namespace Assets {
@@ -34,7 +34,7 @@ namespace TrenchBroom {
         const String EntityDefinitionGroup::displayName() const {
             if (m_name.empty())
                 return "Misc";
-            return StringUtils::capitalize(m_name);
+            return kdl::str_capitalize(m_name);
         }
 
         const std::vector<EntityDefinition*>& EntityDefinitionGroup::definitions() const {

diff --git a/common/src/Assets/ModelDefinition.cpp b/common/src/Assets/ModelDefinition.cpp
@@ -19,15 +19,18 @@
 
 #include "ModelDefinition.h"
 
-#include "StringUtils.h"
 #include "EL/ELExceptions.h"
 #include "EL/EvaluationContext.h"
 #include "EL/Types.h"
 #include "EL/Value.h"
 #include "Model/EntityAttributesVariableStore.h"
 
+#include <kdl/string_compare.h>
+
 #include <vecmath/scalar.h>
 
+#include <ostream>
+
 namespace TrenchBroom {
     namespace Assets {
         ModelSpecification::ModelSpecification() :
@@ -140,7 +143,7 @@ namespace TrenchBroom {
             if (value.type() != EL::ValueType::String)
                 return IO::Path();
             const String& path = value.stringValue();
-            return IO::Path(StringUtils::isPrefix(path, ":") ? path.substr(1) : path);
+            return IO::Path(kdl::cs::is_prefix(path, ":") ? path.substr(1) : path);
         }
 
         size_t ModelDefinition::index(const EL::Value& value) const {

diff --git a/common/src/Assets/Palette.cpp b/common/src/Assets/Palette.cpp
@@ -20,12 +20,13 @@
 #include "Palette.h"
 
 #include "Exceptions.h"
-#include "StringUtils.h"
 #include "IO/File.h"
 #include "IO/Reader.h"
 #include "IO/FileSystem.h"
 #include "IO/ImageLoader.h"
 
+#include <kdl/string_format.h>
+
 #include <algorithm>
 
 namespace TrenchBroom {
@@ -56,7 +57,7 @@ namespace TrenchBroom {
             try {
                 auto file = fs.openFile(path);
                 auto reader = file->reader().buffer();
-                const auto extension = StringUtils::toLower(path.extension());
+                const auto extension = kdl::str_to_lower(path.extension());
                 if (extension == "lmp") {
                     return loadLmp(reader);
                 } else if (extension == "pcx") {

diff --git a/common/src/Assets/Palette.h b/common/src/Assets/Palette.h
@@ -107,7 +107,7 @@ namespace TrenchBroom {
                     }
 
                     for (size_t i = 0; i < 3; ++i) {
-                        averageColor[i] = static_cast<float>(avg[i] / pixelCount / 0xFF);
+                        averageColor[i] = static_cast<float>(avg[i] / static_cast<double>(pixelCount) / static_cast<double>(0xFF));
                     }
                     averageColor[3] = 1.0f;
 

diff --git a/common/src/Assets/TextureManager.cpp b/common/src/Assets/TextureManager.cpp
@@ -24,9 +24,9 @@
 #include "Assets/Texture.h"
 #include "Assets/TextureCollection.h"
 #include "IO/TextureLoader.h"
-#include "StringUtils.h"
 
 #include <kdl/map_utils.h>
+#include <kdl/string_format.h>
 #include <kdl/vector_utils.h>
 
 #include <algorithm>
@@ -144,7 +144,7 @@ namespace TrenchBroom {
         }
 
         Texture* TextureManager::texture(const String& name) const {
-            auto it = m_texturesByName.find(StringUtils::toLower(name));
+            auto it = m_texturesByName.find(kdl::str_to_lower(name));
             if (it == std::end(m_texturesByName)) {
                 return nullptr;
             } else {
@@ -188,7 +188,7 @@ namespace TrenchBroom {
 
             for (auto* collection : m_collections) {
                 for (auto* texture : collection->textures()) {
-                    const auto key = StringUtils::toLower(texture->name());
+                    const auto key = kdl::str_to_lower(texture->name());
                     texture->setOverridden(false);
 
                     auto mIt = m_texturesByName.find(key);

diff --git a/common/src/Color.cpp b/common/src/Color.cpp
@@ -51,13 +51,13 @@ Color::Color(const Color& color, const float a) :
 vec<float, 4>(color.r(), color.g(), color.b(), a) {}
 
 Color::Color(const unsigned char r, const unsigned char g, const unsigned char b, const unsigned char a) :
-vec<float, 4>(r / 255.0f, g / 255.0f, b / 255.0f, a / 255.0f) {}
+vec<float, 4>(static_cast<float>(r) / 255.0f, static_cast<float>(g) / 255.0f, static_cast<float>(b) / 255.0f, static_cast<float>(a) / 255.0f) {}
 
 Color::Color(const int r, const int g, const int b, const int a) :
-vec<float, 4>(r / 255.0f, g / 255.0f, b / 255.0f, a / 255.0f) {}
+vec<float, 4>(static_cast<float>(r) / 255.0f, static_cast<float>(g) / 255.0f, static_cast<float>(b) / 255.0f, static_cast<float>(a) / 255.0f) {}
 
 Color::Color(const int r, const int g, const int b, const float a) :
-vec<float, 4>(r / 255.0f, g / 255.0f, b / 255.0f, a) {}
+vec<float, 4>(static_cast<float>(r) / 255.0f, static_cast<float>(g) / 255.0f, static_cast<float>(b) / 255.0f, a) {}
 
 float Color::r() const {
     return x();

diff --git a/common/src/EL/Value.cpp b/common/src/EL/Value.cpp
@@ -21,10 +21,11 @@
 
 #include "EL/ELExceptions.h"
 #include "StringStream.h"
-#include "StringUtils.h"
 
 #include <kdl/collection_utils.h>
 #include <kdl/map_utils.h>
+#include <kdl/string_compare.h>
+#include <kdl/string_format.h>
 #include <kdl/vector_utils.h>
 
 #include <algorithm>
@@ -104,7 +105,7 @@ namespace TrenchBroom {
                 case ValueType::String:
                     return true;
                 case ValueType::Number: {
-                    if (StringUtils::isBlank(doGetValue()))
+                    if (kdl::str_is_blank(doGetValue()))
                         return true;
                     const char* begin = doGetValue().c_str();
                     char* end;
@@ -127,11 +128,11 @@ namespace TrenchBroom {
         ValueHolder* StringHolder::convertTo(const ValueType toType) const {
             switch (toType) {
                 case ValueType::Boolean:
-                    return new BooleanValueHolder(!StringUtils::caseSensitiveEqual(doGetValue(), "false") && !doGetValue().empty());
+                    return new BooleanValueHolder(!kdl::cs::is_equal(doGetValue(), "false") && !doGetValue().empty());
                 case ValueType::String:
                     return new StringValueHolder(doGetValue());
                 case ValueType::Number: {
-                    if (StringUtils::isBlank(doGetValue()))
+                    if (kdl::str_is_blank(doGetValue()))
                         return new NumberValueHolder(0.0);
                     const char* begin = doGetValue().c_str();
                     char* end;
@@ -153,7 +154,7 @@ namespace TrenchBroom {
 
         void StringHolder::appendToStream(std::ostream& str, const bool /* multiline */, const String& /* indent */) const {
             // Unescaping happens in IO::ELParser::parseLiteral
-            str << "\"" << StringUtils::escape(doGetValue(), "\\\"") << "\"";
+            str << "\"" << kdl::str_escape(doGetValue(), "\\\"") << "\"";
         }
 
 

diff --git a/common/src/IO/Bsp29Parser.cpp b/common/src/IO/Bsp29Parser.cpp
@@ -299,8 +299,8 @@ namespace TrenchBroom {
             if (texture == nullptr) {
                 return vm::vec2f::zero();
             } else {
-                return vm::vec2f((dot(vertex, textureInfo.sAxis) + textureInfo.sOffset) / texture->width(),
-                                 (dot(vertex, textureInfo.tAxis) + textureInfo.tOffset) / texture->height());
+                return vm::vec2f((vm::dot(vertex, textureInfo.sAxis) + textureInfo.sOffset) / static_cast<float>(texture->width()),
+                                 (vm::dot(vertex, textureInfo.tAxis) + textureInfo.tOffset) / static_cast<float>(texture->height()));
             }
         }
     }

diff --git a/common/src/IO/DefParser.cpp b/common/src/IO/DefParser.cpp
@@ -27,6 +27,7 @@
 #include "IO/LegacyModelDefinitionParser.h"
 #include "IO/ParserStatus.h"
 
+#include <kdl/string_format.h>
 #include <kdl/vector_utils.h>
 
 #include <memory>
@@ -218,7 +219,7 @@ namespace TrenchBroom {
             StringList superClasses;
             parseAttributes(status, classInfo, superClasses);
 
-            classInfo.setDescription(StringUtils::trim(parseDescription()));
+            classInfo.setDescription(kdl::str_trim(parseDescription()));
             expect(status, DefToken::CDefinition, token = m_tokenizer.nextToken());
 
             if (classInfo.hasColor()) {

diff --git a/common/src/IO/DiskFileSystem.cpp b/common/src/IO/DiskFileSystem.cpp
@@ -20,7 +20,6 @@
 #include "DiskFileSystem.h"
 
 #include "Exceptions.h"
-#include "StringUtils.h"
 
 #include "IO/DiskIO.h"
 

diff --git a/common/src/IO/DiskIO.cpp b/common/src/IO/DiskIO.cpp
@@ -20,16 +20,17 @@
 #include "DiskIO.h"
 
 #include "Exceptions.h"
-#include "StringUtils.h"
 #include "IO/File.h"
 #include "IO/FileMatcher.h"
 #include "IO/PathQt.h"
 
-#include <QDir>
-#include <QFileInfo>
+#include <kdl/string_compare.h>
 
 #include <fstream>
 
+#include <QDir>
+#include <QFileInfo>
+
 namespace TrenchBroom {
     namespace IO {
         namespace Disk {
@@ -53,7 +54,7 @@ namespace TrenchBroom {
 
             Path findCaseSensitivePath(const Path::List& list, const Path& path) {
                 for (const Path& entry : list) {
-                    if (StringUtils::caseInsensitiveEqual(entry.asString(), path.asString()))
+                    if (kdl::ci::is_equal(entry.asString(), path.asString()))
                         return entry;
                 }
                 return Path("");

diff --git a/common/src/IO/DkPakFileSystem.cpp b/common/src/IO/DkPakFileSystem.cpp
@@ -23,7 +23,8 @@
 #include "IO/Reader.h"
 #include "IO/DiskFileSystem.h"
 #include "IO/IOUtils.h"
-#include "StringUtils.h"
+
+#include <kdl/string_format.h>
 
 #include <cassert>
 #include <cstring>
@@ -111,7 +112,7 @@ namespace TrenchBroom {
                 const auto compressed = reader.readBool<int32_t>();
                 const auto entrySize = compressed ? compressedSize : uncompressedSize;
 
-                const auto entryPath = Path(StringUtils::toLower(entryName));
+                const auto entryPath = Path(kdl::str_to_lower(entryName));
                 auto entryFile = std::make_shared<FileView>(entryPath, m_file, entryAddress, entrySize);
 
                 if (compressed) {