Skip to content

Commit

Permalink
Cleanup: code style
Browse files Browse the repository at this point in the history
  • Loading branch information
@kduske @LogicAndTrick
kduske authored and LogicAndTrick committed Sep 21, 2023
1 parent bd85a4f commit 0d5acb2
Show file tree
Hide file tree
Showing 8 changed files with 1,723 additions and 1,783 deletions.
93 changes: 45 additions & 48 deletions common/src/IO/FgdParser.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,18 @@
#include <kdl/string_utils.h>
#include <kdl/vector_utils.h>

#include <fmt/format.h>

#include <algorithm>
#include <memory>
#include <string>
#include <vector>

namespace TrenchBroom
{
namespace IO
namespace TrenchBroom::IO
{
FgdTokenizer::FgdTokenizer(std::string_view str)
: Tokenizer(std::move(str), "", 0)

FgdTokenizer::FgdTokenizer(const std::string_view str)
: Tokenizer{str, "", 0}
{
}

Expand Down Expand Up @@ -137,8 +138,8 @@ FgdTokenizer::Token FgdTokenizer::emitToken()
e = readUntil(WordDelims);
if (e == nullptr)
{
throw ParserException(
startLine, startColumn, "Unexpected character: '" + std::string(c, 1) + "'");
throw ParserException{
startLine, startColumn, fmt::format("Unexpected character: '{}'", c)};
}
else
{
Expand All @@ -151,11 +152,11 @@ FgdTokenizer::Token FgdTokenizer::emitToken()
}

FgdParser::FgdParser(
std::string_view str,
const std::string_view str,
const Color& defaultEntityColor,
const std::filesystem::path& path)
: EntityDefinitionParser{defaultEntityColor}
, m_tokenizer{FgdTokenizer{std::move(str)}}
, m_tokenizer{FgdTokenizer{str}}
{
if (!path.empty() && path.is_absolute())
{
Expand Down Expand Up @@ -195,22 +196,22 @@ FgdParser::TokenNameMap FgdParser::tokenNames() const
class FgdParser::PushIncludePath
{
private:
FgdParser* m_parser;
FgdParser& m_parser;

public:
PushIncludePath(FgdParser* parser, const std::filesystem::path& path)
PushIncludePath(FgdParser& parser, const std::filesystem::path& path)
: m_parser{parser}
{
m_parser->pushIncludePath(path);
m_parser.pushIncludePath(path);
}

~PushIncludePath() { m_parser->popIncludePath(); }
~PushIncludePath() { m_parser.popIncludePath(); }
};

void FgdParser::pushIncludePath(const std::filesystem::path& path)
void FgdParser::pushIncludePath(std::filesystem::path path)
{
assert(!isRecursiveInclude(path));
m_paths.push_back(path);
m_paths.push_back(std::move(path));
}

void FgdParser::popIncludePath()
Expand All @@ -221,15 +222,8 @@ void FgdParser::popIncludePath()

std::filesystem::path FgdParser::currentRoot() const
{
if (!m_paths.empty())
{
assert(!m_paths.back().empty());
return m_paths.back().parent_path();
}
else
{
return {};
}
assert(m_paths.empty() || !m_paths.back().empty());
return !m_paths.empty() ? m_paths.back().parent_path() : std::filesystem::path{};
}

bool FgdParser::isRecursiveInclude(const std::filesystem::path& path) const
Expand Down Expand Up @@ -300,7 +294,7 @@ std::optional<EntityDefinitionClassInfo> FgdParser::parseClassInfo(ParserStatus&
}
else
{
const auto msg = "Unknown entity definition class '" + classname + "'";
const auto msg = fmt::format("Unknown entity definition class '{}'", classname);
status.error(token.line(), token.column(), msg);
throw ParserException{token.line(), token.column(), msg};
}
Expand Down Expand Up @@ -405,7 +399,7 @@ EntityDefinitionClassInfo FgdParser::parseClassInfo(
status.warn(
token.line(),
token.column(),
"Unknown entity definition header properties '" + typeName + "'");
fmt::format("Unknown entity definition header properties '{}'", typeName));
skipClassProperty(status);
}
token = expect(status, FgdToken::Equality | FgdToken::Word, m_tokenizer.nextToken());
Expand All @@ -418,8 +412,7 @@ EntityDefinitionClassInfo FgdParser::parseClassInfo(
if (token.type() == FgdToken::Colon)
{
m_tokenizer.nextToken();
const auto description = parseString(status);
classInfo.description = kdl::str_trim(description);
classInfo.description = kdl::str_trim(parseString(status));
}

classInfo.propertyDefinitions = parsePropertyDefinitions(status);
Expand Down Expand Up @@ -502,8 +495,9 @@ Assets::ModelDefinition FgdParser::parseModel(ParserStatus& status)
status.warn(
line,
column,
"Legacy model expressions are deprecated, replace with '" + expression.asString()
+ "'");
fmt::format(
"Legacy model expressions are deprecated, replace with '{}'",
expression.asString()));
return Assets::ModelDefinition{expression};
}
catch (const ParserException&)
Expand Down Expand Up @@ -616,7 +610,9 @@ FgdParser::PropertyDefinitionList FgdParser::parsePropertyDefinitions(
if (!addPropertyDefinition(propertyDefinitions, std::move(propertyDefinition)))
{
status.warn(
line, column, "Skipping duplicate property definition: '" + propertyKey + "'");
line,
column,
fmt::format("Skipping duplicate property definition: '{}'", propertyKey));
}

token = expect(status, FgdToken::Word | FgdToken::CBracket, m_tokenizer.nextToken());
Expand Down Expand Up @@ -664,12 +660,8 @@ FgdParser::PropertyDefinitionPtr FgdParser::parsePropertyDefinition(
status.debug(
line,
column,
kdl::str_to_string(
"Unknown property definition type '",
typeName,
"' for property '",
propertyKey,
"'"));
fmt::format(
"Unknown property definition type '{}' for property '{}'", typeName, propertyKey));
return parseUnknownPropertyDefinition(status, propertyKey);
}

Expand Down Expand Up @@ -926,7 +918,9 @@ std::optional<float> FgdParser::parseDefaultFloatValue(ParserStatus& status)
if (token.type() != FgdToken::String)
{
status.warn(
token.line(), token.column(), "Unquoted float default value " + token.data());
token.line(),
token.column(),
fmt::format("Unquoted float default value {}", token.data()));
}
return token.toFloat<float>();
}
Expand Down Expand Up @@ -1033,8 +1027,7 @@ std::vector<EntityDefinitionClassInfo> FgdParser::parseInclude(ParserStatus& sta
assert(kdl::ci::str_is_equal(token.data(), "@include"));

expect(status, FgdToken::String, token = m_tokenizer.nextToken());
const auto path = std::filesystem::path(token.data());
return handleInclude(status, path);
return handleInclude(status, token.data());
}

std::vector<EntityDefinitionClassInfo> FgdParser::handleInclude(
Expand All @@ -1053,34 +1046,38 @@ std::vector<EntityDefinitionClassInfo> FgdParser::handleInclude(
m_tokenizer.restoreStateAndSource(snapshot);
}};

status.debug(m_tokenizer.line(), "Parsing included file '" + path.string() + "'");
status.debug(
m_tokenizer.line(), fmt::format("Parsing included file '{}'", path.string()));

const auto filePath = currentRoot() / path;
return m_fs->openFile(filePath)
.transform([&](auto file) {
status.debug(
m_tokenizer.line(),
"Resolved '" + path.string() + "' to '" + filePath.string() + "'");
fmt::format("Resolved '{}' to '{}'", path.string(), filePath.string()));

if (isRecursiveInclude(filePath))
{
status.error(
m_tokenizer.line(),
"Skipping recursively included file: " + path.string() + " ("
+ filePath.string() + ")");
fmt::format(
"Skipping recursively included file: {} ({})",
path.string(),
filePath.string()));
return std::vector<EntityDefinitionClassInfo>{};
}

const auto pushIncludePath = PushIncludePath{this, filePath};
const auto pushIncludePath = PushIncludePath{*this, filePath};
auto reader = file->reader().buffer();
m_tokenizer.replaceState(reader.stringView());
return parseClassInfos(status);
})
.transform_error([&](auto e) {
status.error(m_tokenizer.line(), "Failed to parse included file: " + e.msg);
status.error(
m_tokenizer.line(), fmt::format("Failed to parse included file: {}", e.msg));
return std::vector<EntityDefinitionClassInfo>{};
})
.value();
}
} // namespace IO
} // namespace TrenchBroom

} // namespace TrenchBroom::IO
15 changes: 7 additions & 8 deletions common/src/IO/FgdParser.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,15 @@
#include <string>
#include <vector>

namespace TrenchBroom
{
namespace Assets
namespace TrenchBroom::Assets
{
class DecalDefinition;
class ModelDefinition;
} // namespace Assets
} // namespace TrenchBroom::Assets

namespace IO
namespace TrenchBroom::IO
{

struct EntityDefinitionClassInfo;
enum class EntityDefinitionClassType;
class FileSystem;
Expand Down Expand Up @@ -95,7 +94,7 @@ class FgdParser : public EntityDefinitionParser, public Parser<FgdToken::Type>

private:
class PushIncludePath;
void pushIncludePath(const std::filesystem::path& path);
void pushIncludePath(std::filesystem::path path);
void popIncludePath();

std::filesystem::path currentRoot() const;
Expand Down Expand Up @@ -163,5 +162,5 @@ class FgdParser : public EntityDefinitionParser, public Parser<FgdToken::Type>
std::vector<EntityDefinitionClassInfo> handleInclude(
ParserStatus& status, const std::filesystem::path& path);
};
} // namespace IO
} // namespace TrenchBroom

} // namespace TrenchBroom::IO
2 changes: 1 addition & 1 deletion common/src/octree.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ class octree

/**
* Finds every data item in this tree whose bounding box intersects with the given ray
* and retuns a list of those items.
* and returns a list of those items.
*
* @param ray the ray to test
* @return a list containing all found data items
Expand Down
27 changes: 12 additions & 15 deletions common/test/src/Assets/EntityDefinitionTestUtils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,17 +35,15 @@

#include "Catch2.h"

namespace TrenchBroom
{
namespace Assets
namespace TrenchBroom::Assets
{
void assertModelDefinition(
const ModelSpecification& expected,
IO::EntityDefinitionParser& parser,
const std::string& entityPropertiesStr)
{
IO::TestParserStatus status;
std::vector<EntityDefinition*> definitions = parser.parseDefinitions(status);
auto status = IO::TestParserStatus{};
auto definitions = parser.parseDefinitions(status);
CHECK(definitions.size() == 1u);

const auto* definition = definitions[0];
Expand All @@ -64,7 +62,7 @@ void assertModelDefinition(
assert(definition->type() == EntityDefinitionType::PointEntity);

const auto* pointDefinition = dynamic_cast<const PointEntityDefinition*>(definition);
const ModelDefinition& modelDefinition = pointDefinition->modelDefinition();
const auto& modelDefinition = pointDefinition->modelDefinition();
assertModelDefinition(expected, modelDefinition, entityPropertiesStr);
}

Expand All @@ -74,9 +72,9 @@ void assertModelDefinition(
const std::string& entityPropertiesStr)
{
const auto entityPropertiesMap = IO::ELParser::parseStrict(entityPropertiesStr)
.evaluate(EL::EvaluationContext())
.evaluate(EL::EvaluationContext{})
.mapValue();
const auto variableStore = EL::VariableTable(entityPropertiesMap);
const auto variableStore = EL::VariableTable{entityPropertiesMap};
CHECK(actual.modelSpecification(variableStore) == expected);
}

Expand All @@ -85,8 +83,8 @@ void assertDecalDefinition(
IO::EntityDefinitionParser& parser,
const std::string& entityPropertiesStr)
{
IO::TestParserStatus status;
std::vector<EntityDefinition*> definitions = parser.parseDefinitions(status);
auto status = IO::TestParserStatus{};
auto definitions = parser.parseDefinitions(status);
CHECK(definitions.size() == 1u);

const auto* definition = definitions[0];
Expand All @@ -105,7 +103,7 @@ void assertDecalDefinition(
assert(definition->type() == EntityDefinitionType::PointEntity);

const auto* pointDefinition = dynamic_cast<const PointEntityDefinition*>(definition);
const DecalDefinition& modelDefinition = pointDefinition->decalDefinition();
const auto& modelDefinition = pointDefinition->decalDefinition();
assertDecalDefinition(expected, modelDefinition, entityPropertiesStr);
}

Expand All @@ -115,10 +113,9 @@ void assertDecalDefinition(
const std::string& entityPropertiesStr)
{
const auto entityPropertiesMap = IO::ELParser::parseStrict(entityPropertiesStr)
.evaluate(EL::EvaluationContext())
.evaluate(EL::EvaluationContext{})
.mapValue();
const auto variableStore = EL::VariableTable(entityPropertiesMap);
const auto variableStore = EL::VariableTable{entityPropertiesMap};
CHECK(actual.decalSpecification(variableStore) == expected);
}
} // namespace Assets
} // namespace TrenchBroom
} // namespace TrenchBroom::Assets
17 changes: 7 additions & 10 deletions common/test/src/Assets/EntityDefinitionTestUtils.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,12 @@

#include <string>

namespace TrenchBroom
{
namespace IO
namespace TrenchBroom::IO
{
class EntityDefinitionParser;
}

namespace Assets
namespace TrenchBroom::Assets
{
class EntityDefinition;
class ModelDefinition;
Expand Down Expand Up @@ -60,8 +58,8 @@ void assertModelDefinition(
const std::string& templateStr,
const std::string& entityPropertiesStr = "{}")
{
const std::string defStr = kdl::str_replace_every(templateStr, "${MODEL}", modelStr);
Parser parser(defStr, Color(1.0f, 1.0f, 1.0f, 1.0f));
const auto defStr = kdl::str_replace_every(templateStr, "${MODEL}", modelStr);
auto parser = Parser{defStr, Color{1, 1, 1, 1}};
assertModelDefinition(expected, parser, entityPropertiesStr);
}

Expand All @@ -85,9 +83,8 @@ void assertDecalDefinition(
const std::string& templateStr,
const std::string& entityPropertiesStr = "{}")
{
const std::string defStr = kdl::str_replace_every(templateStr, "${DECAL}", decalStr);
Parser parser(defStr, Color(1.0f, 1.0f, 1.0f, 1.0f));
const auto defStr = kdl::str_replace_every(templateStr, "${DECAL}", decalStr);
auto parser = Parser{defStr, Color{1, 1, 1, 1}};
assertDecalDefinition(expected, parser, entityPropertiesStr);
}
} // namespace Assets
} // namespace TrenchBroom
} // namespace TrenchBroom::Assets
Loading

0 comments on commit 0d5acb2

Please sign in to comment.