Skip to content

Optionally write comments to config files #180

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Optionally write comments to config files #180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion examples/c++/example3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ int main(int argc, char **argv)
Setting &root = cfg.getRoot();

// Add some settings to the configuration.
Setting &address = root.add("address", Setting::TypeGroup);
Setting &address = root.add("address", Setting::TypeGroup,
"Address for parcel delivery");

address.add("street", Setting::TypeString) = "1 Woz Way";
address.add("city", Setting::TypeString) = "San Jose";
Expand Down
3 changes: 2 additions & 1 deletion examples/c/example3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ int main(int argc, char **argv)
root = config_root_setting(&cfg);

/* Add some settings to the configuration. */
group = config_setting_add(root, "address", CONFIG_TYPE_GROUP);
group = config_setting_add_with_comment(
root, "address", CONFIG_TYPE_GROUP, "Address for parcel delivery");

setting = config_setting_add(group, "street", CONFIG_TYPE_STRING);
config_setting_set_string(setting, "1 Woz Way");
Expand Down
37 changes: 27 additions & 10 deletions lib/libconfig.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,13 @@ static void __config_write_setting(const config_t *config,
char nongroup_assign_char = config_get_option(
config, CONFIG_OPTION_COLON_ASSIGNMENT_FOR_NON_GROUPS) ? ':' : '=';

if(setting->comment)
{
if(depth > 1)
__config_indent(stream, depth, config->tab_width);
fprintf(stream, "// %s\n", setting->comment);
Copy link
Owner

Choose a reason for hiding this comment

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

If the comment contains newlines, this will result in a syntax error in the generated file. The comment should be written out such that each line begins with //.

}

if(depth > 1)
__config_indent(stream, depth, config->tab_width);

Expand Down Expand Up @@ -824,7 +831,8 @@ void config_set_hook(config_t *config, void *hook)
/* ------------------------------------------------------------------------- */

static config_setting_t *config_setting_create(config_setting_t *parent,
const char *name, int type)
const char *name,
int type, const char *comment)
{
config_setting_t *setting;
config_list_t *list;
Expand All @@ -839,6 +847,7 @@ static config_setting_t *config_setting_create(config_setting_t *parent,
setting->config = parent->config;
setting->hook = NULL;
setting->line = 0;
setting->comment = (comment == NULL) ? NULL : strdup(comment);
Copy link
Owner

Choose a reason for hiding this comment

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

__config_setting_destroy() needs to be updated to free this string, otherwise we have a memory leak


list = parent->value.list;

Expand Down Expand Up @@ -1343,7 +1352,7 @@ config_setting_t *config_setting_set_int_elem(config_setting_t *setting,
if(! __config_list_checktype(setting, CONFIG_TYPE_INT))
return(NULL);

element = config_setting_create(setting, NULL, CONFIG_TYPE_INT);
element = config_setting_create(setting, NULL, CONFIG_TYPE_INT, NULL);
}
else
{
Expand Down Expand Up @@ -1385,7 +1394,7 @@ config_setting_t *config_setting_set_int64_elem(config_setting_t *setting,
if(! __config_list_checktype(setting, CONFIG_TYPE_INT64))
return(NULL);

element = config_setting_create(setting, NULL, CONFIG_TYPE_INT64);
element = config_setting_create(setting, NULL, CONFIG_TYPE_INT64, NULL);
}
else
{
Expand Down Expand Up @@ -1426,7 +1435,7 @@ config_setting_t *config_setting_set_float_elem(config_setting_t *setting,
if(! __config_list_checktype(setting, CONFIG_TYPE_FLOAT))
return(NULL);

element = config_setting_create(setting, NULL, CONFIG_TYPE_FLOAT);
element = config_setting_create(setting, NULL, CONFIG_TYPE_FLOAT, NULL);
}
else
element = config_setting_get_elem(setting, idx);
Expand Down Expand Up @@ -1471,7 +1480,7 @@ config_setting_t *config_setting_set_bool_elem(config_setting_t *setting,
if(! __config_list_checktype(setting, CONFIG_TYPE_BOOL))
return(NULL);

element = config_setting_create(setting, NULL, CONFIG_TYPE_BOOL);
element = config_setting_create(setting, NULL, CONFIG_TYPE_BOOL, NULL);
}
else
element = config_setting_get_elem(setting, idx);
Expand Down Expand Up @@ -1517,7 +1526,7 @@ config_setting_t *config_setting_set_string_elem(config_setting_t *setting,
if(! __config_list_checktype(setting, CONFIG_TYPE_STRING))
return(NULL);

element = config_setting_create(setting, NULL, CONFIG_TYPE_STRING);
element = config_setting_create(setting, NULL, CONFIG_TYPE_STRING, NULL);
}
else
element = config_setting_get_elem(setting, idx);
Expand All @@ -1535,7 +1544,7 @@ config_setting_t *config_setting_set_string_elem(config_setting_t *setting,

config_setting_t *config_setting_get_elem(const config_setting_t *setting,
unsigned int idx)
{
{
config_list_t *list;

if(! config_setting_is_aggregate(setting))
Expand Down Expand Up @@ -1606,8 +1615,8 @@ void config_setting_set_hook(config_setting_t *setting, void *hook)

/* ------------------------------------------------------------------------- */

config_setting_t *config_setting_add(config_setting_t *parent,
const char *name, int type)
config_setting_t *config_setting_add_with_comment(config_setting_t *parent,
const char *name, int type, const char *comment)
{
if((type < CONFIG_TYPE_NONE) || (type > CONFIG_TYPE_LIST))
return(NULL);
Expand Down Expand Up @@ -1635,7 +1644,15 @@ config_setting_t *config_setting_add(config_setting_t *parent,
return(NULL); /* already exists */
}

return(config_setting_create(parent, name, type));
return(config_setting_create(parent, name, type, comment));
}

/* ------------------------------------------------------------------------- */

config_setting_t *config_setting_add(config_setting_t *parent,
const char *name, int type)
{
config_setting_add_with_comment(parent, name, type, NULL);
}

/* ------------------------------------------------------------------------- */
Expand Down
3 changes: 3 additions & 0 deletions lib/libconfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ typedef struct config_setting_t
void *hook;
unsigned int line;
const char *file;
char *comment;
} config_setting_t;

typedef enum
Expand Down Expand Up @@ -290,6 +291,8 @@ extern LIBCONFIG_API config_setting_t *config_setting_get_member(

extern LIBCONFIG_API config_setting_t *config_setting_add(
config_setting_t *parent, const char *name, int type);
extern LIBCONFIG_API config_setting_t *config_setting_add_with_comment(
Copy link
Owner

Choose a reason for hiding this comment

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

We should have a config_setting_set_comment() function as well.

config_setting_t *parent, const char *name, int type, const char *comment);
extern LIBCONFIG_API int config_setting_remove(config_setting_t *parent,
const char *name);
extern LIBCONFIG_API int config_setting_remove_elem(config_setting_t *parent,
Expand Down
2 changes: 1 addition & 1 deletion lib/libconfig.h++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ class LIBCONFIGXX_API Setting

void remove(unsigned int idx);

Setting & add(const char *name, Type type);
Setting & add(const char *name, Type type, const char *comment = NULL);

inline Setting & add(const std::string &name, Type type)
{ return(add(name.c_str(), type)); }
Expand Down
5 changes: 3 additions & 2 deletions lib/libconfigcpp.c++
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ void Setting::remove(unsigned int idx)

// ---------------------------------------------------------------------------

Setting & Setting::add(const char *name, Setting::Type type)
Setting & Setting::add(const char *name, Setting::Type type, const char *comment)
Copy link
Owner

Choose a reason for hiding this comment

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

should have an overload that takes a const std::string &comment (a simple inline function in libconfig.h++)

doc/libconfig.texi needs to be updated with documentation for the new APIs.

Copy link
Owner

Choose a reason for hiding this comment

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

also a couple of unit tests would be nice

{
assertType(TypeGroup);

Expand All @@ -1115,7 +1115,8 @@ Setting & Setting::add(const char *name, Setting::Type type)
if(typecode == CONFIG_TYPE_NONE)
throw SettingTypeException(*this, name);

config_setting_t *setting = config_setting_add(_setting, name, typecode);
config_setting_t *setting = config_setting_add_with_comment(
_setting, name, typecode, comment);

if(! setting)
throw SettingNameException(*this, name);
Expand Down