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

Section containing multiple CFG_STR_LIST()s with the same key? #44

Open
ThomasAdam opened this issue Nov 24, 2015 · 11 comments
Open

Section containing multiple CFG_STR_LIST()s with the same key? #44

ThomasAdam opened this issue Nov 24, 2015 · 11 comments

Comments

@ThomasAdam
Copy link
Contributor

Hi all,

I'm trying to use a section which contains multiple string lists with the same key. Hopefully the following will illustrate how I'm trying to achieve that:

cfg_opt_t    bind_opts[] = {
    CFG_STR_LIST("key", "{CM-Return, terminal}", CFGF_NONE),
    CFG_STR_LIST("key", "{CM-Delete, lock}", CFGF_NONE),
    CFG_STR_LIST("key", "{M-question, exec}", CFGF_NONE),
    CFG_STR_LIST("key", "{M-period, ssh}", CFGF_NONE),
    CFG_STR_LIST("key", "{M-Return, hide}", CFGF_NONE),
        CFG_END()
};

cfg_opt_t    all_cfg_opts[] = {
    CFG_SEC("bindings", bind_opts, CFGF_MULTI),
        CFG_END()
};

Certainly, if I then do something like this from within my program:

cfg_t    *sec = cfg_getsec(cfg, "bindings");
cfg_print(sec, stdout);

I see:

bindings {
  key = {"CM-Return", "terminal"}
  key = {"CM-Delete", "lock"}
  key = {"M-question", "exec"}
  key = {"M-period", "ssh"}
  key = {"M-Return", "hide"}
};

Which suggests libconfuse is happy, yet when I do something like this:

fprintf(stderr, "size of bindings is: %d\n", cfg_size(bind_sec, "bindings"));

It prints the value of 1 when I expect to see 5.

Am I doing something wrong, or does libconfuse simply not support this type of structure, and is flattening down all of the key = lines into one?

Any help, greatly appreciated!

@troglobit
Copy link
Collaborator

Good question! I have to check that myself first before I can answer. In the meantime, what version of libConfuse are you using?

@ThomasAdam
Copy link
Contributor Author

Version 2.7, but I tried Git last night and there's no difference, from what I can tell.

@peda-r
Copy link
Contributor

peda-r commented Nov 24, 2015

Hi Thomas,

On 2015-11-24 11:07, Thomas Adam wrote:

Hi all,

I'm trying to use a section which contains multiple string lists with the same key. Hopefully the following will illustrate how I'm trying to achieve that:

|cfg_opt_t bind_opts[] = {
CFG_STR_LIST("key", "{CM-Return, terminal}", CFGF_NONE),
CFG_STR_LIST("key", "{CM-Delete, lock}", CFGF_NONE),
CFG_STR_LIST("key", "{M-question, exec}", CFGF_NONE),
CFG_STR_LIST("key", "{M-period, ssh}", CFGF_NONE),
CFG_STR_LIST("key", "{M-Return, hide}", CFGF_NONE),
CFG_END()
};

cfg_opt_t all_cfg_opts[] = {
CFG_SEC("bindings", bind_opts, CFGF_MULTI),
CFG_END()
};

Certainly, if I then do something like this from within my program:

|
|

cfg_t *sec = cfg_getsec(cfg, "bindings");
cfg_print(sec, stdout);

|
|

|
|

bindings {
key = {"CM-Return", "terminal"}
key = {"CM-Delete", "lock"}
key = {"M-question", "exec"}
key = {"M-period", "ssh"}
key = {"M-Return", "hide"}
};

|
|

|
|

fprintf(stderr, "size of bindings is: %d\n", cfg_size(bind_sec, "bindings"));

|
|

Am I doing something wrong, or does libconfuse simply not support this type of structure, and is flattening down all of the key = lines into one?

Any help, greatly appreciated!
|

As you guessed, libconfuse does not support that. You need to either create a
whole section for each binding, i.e.

binding "CM-Return" {
function = "terminal"
}
binding "CM-Delete" {
function = "lock"
}
etc (which is kind of bulky...)

or use two lists of strings, i.e.

binding-key = {"CM-Return", "CM-Delete", "M-question", "M-period", "M-Return"}
binding-func = {"terminal", "lock", "exec", "ssh", "hide"}

(but this one is a bit unreadable if you have long lists)

Or perhaps something else?

Cheers,
Peter

@ThomasAdam
Copy link
Contributor Author

Hi Peter,

Ah, that's a shame. It's interesting how libconfuse lulls you into a false sense of security, since it seems quite happy with the initial structure. :)

Would you, or the project, perhaps accept a patch to allow this structure to be supported? If so, I'd ne happy to do that, and to discuss what the semantics internally should look like to support it. I certainly don't like the alternatives, as they're too verbose and don't convey the relationship in the same way that my original example does.

@peda-r
Copy link
Contributor

peda-r commented Nov 24, 2015

The thing is that options that are lists of values internally use the same mechanism that sections use when you have multiple sections, i.e. a list of sections for an option instead of a list of values for an option. So, (I assume) you will have to do some major restructuring to support options harboring lists of lists.

But an early failure when you cfg_init a bad config template wouldn't be wrong, I suppose.

(I'm not in a position to accept or reject anything in this repo, I'm just a random user)

@ThomasAdam
Copy link
Contributor Author

I don't consider that a problem, and having looked into this, I don't think it's too much work at all. The only concern I have is that it might be seen as a short cut to not using sections.

I'll put together a set of patches and see what happens next. If anyone has any thoughts or things they'd like to see from this in the meantime, feel free to speak up, otherwise watch this space!

@troglobit
Copy link
Collaborator

@ThomasAdam Looking forward to your proposal! 👍

@peda-r
Copy link
Contributor

peda-r commented Nov 25, 2015

@ThomasAdam consider that cfg_getopt (and other functions too) has to work for looking up the option, so I do not think it is going to be as simple as allowing several option instances with the same name.

But maybe you see something that I do not...

@ThomasAdam
Copy link
Contributor Author

@peda-r - yup, this wasn't going to be trivial, but it doesn't mean it can't be done. We'll see.

@peda-r
Copy link
Contributor

peda-r commented Nov 25, 2015

Yep, good luck!

Meanwhile, I cooked up #45 so that you get an early error instead of a false hope...

@LonerDan
Copy link
Contributor

I see this is a quite old and still open issue with no pull requests, so I am not sure what happened here, but I just wanted to chime in with some thoughts.
I think that the approach using titled sections is quite elegant and readable, albeit it is a little bit more verbose and "heavyweight" I suppose. On the other hand, it could work well for sections scattered across multiple config files included by the central one.
Another possible solution for @ThomasAdam's problem would be the use of a section with the KEYSTRVAL flag. Example:

cfg_opt_t all_cfg_opts[] = {
    CFG_SEC("bindings", NULL, CFGF_KEYSTRVAL),
    CFG_END()
};
bindings {
    CM-Return = "terminal"
    CM-Delete = "lock"
    M-question = "exec"
    M-period = "ssh"
    M-Return = "hide"
}

In general, I don't think this is much of an issue regarding the library itself, but rather an issue of unfortunate/inefficient design of the way @ThomasAdam wants his data represented in the config file.
I personally think that if some uniquely identifiable entry (be it an option key or section with a title) is specified multiple times in the config file, only the latest mention of it should be taken. This behavior is beneficial when the main config file includes one or more other configuration files, which then can override the default/system-wide options in that main file (this is IMHO common patter in Linux programs, where user-defined config files can override/change the system level config). If needed, this could be made possible to be explicitly changed by the use of the CFGF_MULTI flag.
What I see as potentially a beneficial new feature is the addition of a map data type to complement the existing list, although one could argue there is some overlap with what can be achieved with the KEYSTRVAL sections.

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

No branches or pull requests

4 participants