Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic support for lists in config.yaml #342

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 25 additions & 22 deletions FluidNC/src/Configuration/GenericFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@
#include "HandlerBase.h"

namespace Configuration {
template <typename BaseType>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to make BuilderBase a non-inner class to be able to forward declare it in HandlerBase.h

class BuilderBase {
const char* name_;

public:
BuilderBase(const char* name) : name_(name) {}

BuilderBase(const BuilderBase& o) = delete;
BuilderBase& operator=(const BuilderBase& o) = delete;

virtual BaseType* create() const = 0;
const char* name() const { return name_; }

virtual ~BuilderBase() = default;
};

template <typename BaseType>
class GenericFactory {
static GenericFactory& instance() {
Expand All @@ -20,30 +36,15 @@ namespace Configuration {
GenericFactory(const GenericFactory&) = delete;
GenericFactory& operator=(const GenericFactory&) = delete;

class BuilderBase {
const char* name_;

public:
BuilderBase(const char* name) : name_(name) {}

BuilderBase(const BuilderBase& o) = delete;
BuilderBase& operator=(const BuilderBase& o) = delete;

virtual BaseType* create() const = 0;
const char* name() const { return name_; }

virtual ~BuilderBase() = default;
};

std::vector<BuilderBase*> builders_;
std::vector<BuilderBase<BaseType>*> builders_;

inline static void registerBuilder(BuilderBase* builder) { instance().builders_.push_back(builder); }
inline static void registerBuilder(BuilderBase<BaseType>* builder) { instance().builders_.push_back(builder); }

public:
template <typename DerivedType>
class InstanceBuilder : public BuilderBase {
class InstanceBuilder : public BuilderBase<BaseType> {
public:
InstanceBuilder(const char* name) : BuilderBase(name) { instance().registerBuilder(this); }
InstanceBuilder(const char* name) : BuilderBase<BaseType>(name) { instance().registerBuilder(this); }

BaseType* create() const override { return new DerivedType(); }
};
Expand All @@ -66,9 +67,11 @@ namespace Configuration {
if (handler.handlerType() == HandlerType::Parser) {
for (auto it : instance().builders_) {
if (handler.matchesUninitialized(it->name())) {
auto product = it->create();
inst.push_back(product);
handler.enterFactory(it->name(), *product);
std::vector<Configurable*> instlocal;
handler.enterFactoryList(it->name(), it, instlocal);
for (const auto& value : instlocal) {
inst.push_back((BaseType*)value);
}

return;
}
Expand Down
15 changes: 13 additions & 2 deletions FluidNC/src/Configuration/HandlerBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@ namespace Configuration {
template <typename BaseType>
class GenericFactory;

template <typename BaseType>
class BuilderBase;

class HandlerBase {
protected:
virtual void enterSection(const char* name, Configurable* value) = 0;
virtual bool matchesUninitialized(const char* name) = 0;
virtual void enterSectionList(const char* name, BuilderBase<Configurable>* builder, std::vector<Configurable*>& inst) {
throw "WIP: Won't work";
};
virtual void enterSection(const char* name, Configuration::Configurable* value) = 0;
virtual bool matchesUninitialized(const char* name) = 0;

template <typename BaseType>
friend class GenericFactory;
Expand Down Expand Up @@ -77,6 +83,11 @@ namespace Configuration {
}
}

template <typename T>
void enterFactoryList(const char* name, BuilderBase<T>* value, std::vector<Configuration::Configurable*>& inst) {
enterSectionList(name, (BuilderBase<Configurable>*)value, inst);
}

template <typename T>
void enterFactory(const char* name, T& value) {
enterSection(name, &value);
Expand Down
30 changes: 29 additions & 1 deletion FluidNC/src/Configuration/ParserHandler.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,40 @@ namespace Configuration {
std::vector<const char*> _path;

public:
void enterSectionList(const char* name, BuilderBase<Configurable>* builder, std::vector<Configurable*>& inst) override {
int entryIndent = _parser.token_.indent_;
// check if list
_parser.Tokenize();
_parser.token_.state = TokenState::Held;
if (!_parser.token_.is_list_) {
#ifdef DEBUG_CHATTY_YAML_PARSER
log_debug("----------- Entered single item under " << name << " at indent " << _parser.token_.indent_);
#endif
inst.push_back(builder->create());
this->enterSectionItem(name, inst.back(), entryIndent);
}

while (_parser.token_.is_list_) {
_parser.token_.is_list_ = false;
entryIndent = _parser.token_.indent_;
_parser.token_.indent_ += 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks a bit fishy. I could introduce another member on Token as well, but it would basically do the same, I assume.

#ifdef DEBUG_CHATTY_YAML_PARSER
log_debug("----------- Entered list item under " << name << " at indent " << _parser.token_.indent_);
#endif
inst.push_back(builder->create());
this->enterSectionItem(name, inst.back(), entryIndent);
}
}

void enterSection(const char* name, Configuration::Configurable* section) override {
this->enterSectionItem(name, section, _parser.token_.indent_);
}

void enterSectionItem(const char* name, Configuration::Configurable* section, int entryIndent) {
_path.push_back(name); // For error handling

// On entry, the token is for the section that invoked us.
// We will handle following nodes with indents greater than entryIndent
int entryIndent = _parser.token_.indent_;
#ifdef DEBUG_CHATTY_YAML_PARSER
log_debug("Entered section " << name << " at indent " << entryIndent);
#endif
Expand Down
11 changes: 11 additions & 0 deletions FluidNC/src/Configuration/Tokenizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,17 @@ namespace Configuration {
goto parseAgain;

default:
if (IsListItem()) {
token_.is_list_ = true;
Inc();
// skip expected whitespace. TODO: validate that it's indeed a whitespace
Copy link
Contributor Author

@nils nils Mar 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reason: It could also be a plain scalar (e.g. -42)
But did this YAML parser support that before? I guess not, it always relied on "key: value" pairs.

Inc();

#ifdef DEBUG_VERBOSE_YAML_TOKENIZER
log_debug("List token");
#endif
}

if (!IsIdentifierChar()) {
ParseError("Invalid character");
}
Expand Down
6 changes: 5 additions & 1 deletion FluidNC/src/Configuration/Tokenizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ namespace Configuration {
return c == ' ' || c == '\t' || c == '\f' || c == '\r';
}

inline bool IsListItem() { return Current() == '-'; }

inline bool IsIdentifierChar() { return IsAlpha() || IsDigit() || Current() == '_'; }

inline bool IsEndLine() { return Eof() || Current() == '\n'; }
Expand Down Expand Up @@ -69,11 +71,13 @@ namespace Configuration {
// is called to handle the top level of the YAML config file, tokens at
// indent 0 will be processed.
TokenData() :
keyStart_(nullptr), keyEnd_(nullptr), indent_(-1), state(TokenState::Bof), sValueStart_(nullptr), sValueEnd_(nullptr) {}
keyStart_(nullptr), keyEnd_(nullptr), indent_(-1), is_list_(false), state(TokenState::Bof), sValueStart_(nullptr),
sValueEnd_(nullptr) {}

const char* keyStart_;
const char* keyEnd_;
int indent_;
bool is_list_;

TokenState state = TokenState::Bof;

Expand Down