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

Add function to create a new titled section #69

Merged
merged 4 commits into from May 27, 2016

Conversation

jonasj76
Copy link
Contributor

Example included.

Signed-off-by: Jonas Johansson <jonasj76@gmail.com>
Signed-off-by: Jonas Johansson <jonasj76@gmail.com>
@troglobit
Copy link
Collaborator

Great stuff, Jonas! Could you perhaps also add a unit test (see tests/) for this?

@peda-r
Copy link
Contributor

peda-r commented May 25, 2016

I have been using:
char tmp[large_enough];
sprintf(tmp, "section-name \"%s\" {\n}", title);
cfg_parse_buf(cfg, tmp);

Which is kind of ugly...

@peda-r
Copy link
Contributor

peda-r commented May 25, 2016

Hmm, I think attempts to add a pre-existing section should fail.
Maybe add a flags argument to get selective behavior?

@troglobit
Copy link
Collaborator

@peda-r Yeah this is cleaner.

Hmm, you're right and it also touches upon a related problem of mine. A global CFGF_OVERWRITE flag could control overwriting anything, good idea!

@peda-r
Copy link
Contributor

peda-r commented May 25, 2016

I was thinking a flags argument to the new function, but I guess I don't care deeply about where the flags are specified...

@troglobit
Copy link
Collaborator

I just zoomed out a bit: a user that wants to add a section likely also want to be able to add other things as well.

Signed-off-by: Jonas Johansson <jonasj76@gmail.com>
@jonasj76
Copy link
Contributor Author

Unit test added.
I agree that the default for adding a pre-existing section should fail. But should it be a flag or not to override this behaviour?

@troglobit
Copy link
Collaborator

troglobit commented May 25, 2016

@jonasj76 Thanks! :)

Yeah I think we should have a global flag, like this:

if (cfg->flags & CFGF_OVERWRITE)
    clear_current_opt()
if (set_value() fails)
     errno = EEXIST;
     return NULL;

or something, but I guess it could also be an option flag. I don't have a strong opinion on either one. With a global flag we could let it cascade down on all options on cfg_init() so the code only needs to check opt->flags ...

Signed-off-by: Jonas Johansson <jonasj76@gmail.com>
@jonasj76
Copy link
Contributor Author

Added commit to fail when adding a pre-existing section.

@troglobit
Copy link
Collaborator

Awesome, thanks!

Like we discussed AFK today, adding a global and/or per-setting CFGF_OVERWRITE can be added later.

@troglobit troglobit merged commit 66507e5 into libconfuse:master May 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants