add add-feature #5

Closed
wants to merge 1 commit into
from

Projects

None yet

2 participants

@bertvannuffelen

Hi,

I've added a small extension in which one can add a value to a comma separated value list.

e.g.

[Section]
DirsAllowed = /home/me, /etc/app
crudini --set --add my.ini Section DirsAllowed "/home/other"

will result into

[Section]
DirsAllowed = /home/me, /etc/app, /home/other
Bert Van Nuffelen add add-feature
add the feature to add extra values to an existing parameter.
18b7823
@pixelb

I don't think --add should be concerned with the above.
I.E. it should just create new sections if --existing is not specified

fine with me.

@pixelb

Consequently this could be just moved down before the conf.get() below.
Since _val may be empty I'd support adding value without a ',' in that case.
Also if value is already in the list should should we add it again?

on the empty case, ok

on the if the value exists in the list case: probably in the case of ini configuration files multiple entries of the same value are not so frequent and meaningfull. So it would be fine to consider the list as a set.

@pixelb

--add is a bit restrictive/confusing.
How about using --list to indicate we're manipulating a list of values.
In future we might support params to --list to have different delimiters etc.

Hi Padraig,

list is fine. support for other deliminitors would be great. I have tought of that too, but for the use case I am concerned with a comma is sufficient now.

kind regards,

Bert

We could also consider the option separated from --set and create a --add_list option.

It would be better separated since it's pertaining to value contents rather than the --set action.
So I suppose you could just have a --list option which will update a set of values with --set,
will delete items with --del (which would be updated to accept a value), and possibly format values for output with --get

ok.
Is the next you propose to implement?

cmd + " --list [--existing] config_file section param value"

Right.

@pixelb
Owner
pixelb commented Mar 19, 2014

So as a general point, this is getting in the format of the values which is dangerous territory,
which we should only consider if very useful. For some other notes on lists in ini files see:
http://stackoverflow.com/questions/335695/lists-in-configparser

Now this does seem useful we just have to be careful

@pixelb
Owner
pixelb commented Sep 3, 2014

Done in commit 42673cc

@pixelb pixelb closed this Sep 3, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment