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.

Sign up for GitHub

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

Jump to bottom

IO: add support for reading/writing XML attributes #267

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

IO: add support for reading/writing XML attributes #267

wants to merge 2 commits into from

Conversation

andrea-iob
Copy link
Member

The class ConfigParser is now able to read/write XML attributes. XML attributes are designed to contain data related to a specific element, for example here:

<x min="0" max="100">11.11</x>

min and max are two attributes of the element x.

I tried to keep the interface for interacting with the attributes as close as possible to the existing interface to interact with options. Attribute can be get/set using the following functions (and their corresponding templates):

    std::string getAttribute(const std::string &key, const std::string &name) const;
    std::string getAttribute(const std::string &key, const std::string &name, const std::string &fallback) const;
    const Attributes & getAttributes(const std::string &key) const;
    void setAttribute(const std::string &key, const std::string &name, const std::string &value);

Options stored in the configuration are now a struct and not a string anymore. This break compatibility with previous versions, however this is a problem only if the function "getOptions" was used. The struct stores both the option value an its attributes. There are new function to add/get Option objects:

    Option & getOption(const std::string &key);
    const Option & getOption(const std::string &key) const;
    void addOption(const std::string &key, const Option &option);
    void addOption(const std::string &key, Option &&option);
    void addOption(const std::string &key, const std::string &value);
    void addOption(const std::string &key, std::string &&value);
    void addOption(const std::string &key, const std::string &value, const Attributes &attributes);
    void addOption(const std::string &key, std::string &&value, Attributes &&attributes);

@andrea-iob andrea-iob self-assigned this Feb 9, 2022
@roccoarpa
Copy link
Contributor

Hi, I had a very quick look, and honestly I need some time to study better the solution you proposed and test it from my side.
Meanwhile, two remarks:

  1. why Attributes are considered attached only on value cases (leaf element of the xml tree) and not on Sections (see below an example of what i meant )
  2. json does not support attributes as xml: it considers attributes as regular leaf element of the tree. If we want to have 1:1 reversibility between the 2 formas, we need a special distinctive mark(maybe a @ before the name of each element in the json file ) to absorb some element of the json tree as attributes of the Config tree and viceversa to flush.

Anyway, give me some time please, I can came up with a more consistent and clear review to make my point. Meanwhile a xml example of what I meant in 1) :

<messages>
  <note id="501">
    <to internal="yes">Tove</to>
    <from internal="yes">Jani</from>
    <heading>Reminder</heading>
    <body font="Tahoma">Don't forget me this weekend!</body>
  </note>
  <note id="502">
    <to internal="yes">Jani</to>
    <from internal="yes">Tove</from>
    <heading>Re: Reminder</heading>
    <body font="Calibri">I will not</body>
  </note>
</messages>

@roccoarpa roccoarpa mentioned this pull request May 31, 2022
Closed
@andrea-iob andrea-iob mentioned this pull request Sep 23, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edoardolombardi edoardolombardi Awaiting requested review from edoardolombardi

@marcocisternino marcocisternino Awaiting requested review from marcocisternino

@roccoarpa roccoarpa Awaiting requested review from roccoarpa

At least 1 approving review is required to merge this pull request.

Assignees

@andrea-iob andrea-iob

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrea-iob @roccoarpa