Special section "DEFAULT" #1

Merged
merged 4 commits into from Jul 23, 2011

Conversation

Projects
None yet
2 participants
@pivaldi
Contributor

pivaldi commented Jul 17, 2011

Special section "DEFAULT" sets default values for the other sections like Python does : http://docs.python.org/library/configparser.html

@Ajnasz

This comment has been minimized.

Show comment
Hide comment
@Ajnasz

Ajnasz Jul 18, 2011

Owner

How would you deal when you save a configuration file.
With your current solution the default values would be duplicated if you would modify a configuration and save it.

Owner

Ajnasz commented Jul 18, 2011

How would you deal when you save a configuration file.
With your current solution the default values would be duplicated if you would modify a configuration and save it.

@pivaldi

This comment has been minimized.

Show comment
Hide comment
@pivaldi

pivaldi Jul 18, 2011

Contributor

Hi,

You're right, I had not thought about saving. What do you think about modifying getBlock and getParam instead ?
If so, what do you think about to add a parameter "raw" to these functions in order to provide an output without interference by the section "DEFAULT" ?

PS : sorry for my poor English, it's not my natural language.

Contributor

pivaldi commented Jul 18, 2011

Hi,

You're right, I had not thought about saving. What do you think about modifying getBlock and getParam instead ?
If so, what do you think about to add a parameter "raw" to these functions in order to provide an output without interference by the section "DEFAULT" ?

PS : sorry for my poor English, it's not my natural language.

@Ajnasz

This comment has been minimized.

Show comment
Hide comment
@Ajnasz

Ajnasz Jul 20, 2011

Owner

I did some changes. The getBlock became deprecated, basically it did the same as getParam but now it does it fully.

So, you should put the stuff into the getParam method, but I would suggest to use a config parameter (like async) instead of passing another argument the function. The reason is why I suggest it is because if we don't do this, it would make the param method more complicated. You would need to add another parameter to it which would be useless in setParam.

What do you think?

Owner

Ajnasz commented Jul 20, 2011

I did some changes. The getBlock became deprecated, basically it did the same as getParam but now it does it fully.

So, you should put the stuff into the getParam method, but I would suggest to use a config parameter (like async) instead of passing another argument the function. The reason is why I suggest it is because if we don't do this, it would make the param method more complicated. You would need to add another parameter to it which would be useless in setParam.

What do you think?

@pivaldi

This comment has been minimized.

Show comment
Hide comment
@pivaldi

pivaldi Jul 20, 2011

Contributor

What do you think?
I think that's good.
Take a look to my last commit.

Contributor

pivaldi commented Jul 20, 2011

What do you think?
I think that's good.
Take a look to my last commit.

@Ajnasz

This comment has been minimized.

Show comment
Hide comment
@Ajnasz

Ajnasz Jul 21, 2011

Owner

Looks really cool. :)

Check out what happens, if you add a test case like this:

assert.deepEqual(obj.param().foo.test_default, 'I come from the default section',
'test_default's key value in foo is not inherited from DEFAULT section'
);

// Hint
The param() will return the full values object if you call it without
any parameter;). I left this there because of backward compatibility (the getBlock did that).

Lajos Koszti

Owner

Ajnasz commented Jul 21, 2011

Looks really cool. :)

Check out what happens, if you add a test case like this:

assert.deepEqual(obj.param().foo.test_default, 'I come from the default section',
'test_default's key value in foo is not inherited from DEFAULT section'
);

// Hint
The param() will return the full values object if you call it without
any parameter;). I left this there because of backward compatibility (the getBlock did that).

Lajos Koszti

@pivaldi

This comment has been minimized.

Show comment
Hide comment
@pivaldi

pivaldi Jul 21, 2011

Contributor

Fixed :)

Contributor

pivaldi commented Jul 21, 2011

Fixed :)

@Ajnasz Ajnasz merged commit 618816f into Ajnasz:master Jul 23, 2011

@Ajnasz

This comment has been minimized.

Show comment
Hide comment
@Ajnasz

Ajnasz Jul 23, 2011

Owner

Merged your changes and also published to npm.

Thanks for the patches.

Owner

Ajnasz commented Jul 23, 2011

Merged your changes and also published to npm.

Thanks for the patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment