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

From 618816f, implement interpolation of %([BLOCK.]KEY) #2

Closed
wants to merge 1 commit into from
Closed

From 618816f, implement interpolation of %([BLOCK.]KEY) #2

wants to merge 1 commit into from

Conversation

pivaldi
Copy link
Contributor

@pivaldi pivaldi commented Jul 24, 2011

On my branch "optimized"…
I've reverted to commit 618816f because the commit bcb7c67 disallows an optimized implementation of interpolation of %(block.key), see comment on the commit bcb7c67.
I've implemented the extension of %(block.key) like the Python ConfigParser does but %(REF) can refer to an other block and can be recursive.

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 24, 2011

Check out my interpolation branch 9c7e9b6

How differs the getParam from yours:
First I'm working with the params like when the inheritDefault is false. Then if it's true and the values has the DEFAULT section, I update the values. The updating is moved out into a separate method, so it's easier to read to me.

@pivaldi
Copy link
Contributor Author

pivaldi commented Jul 24, 2011

It looks perfect !

@pivaldi
Copy link
Contributor Author

pivaldi commented Jul 24, 2011

In fact, in your branch interpolation, parser.interpolate() does not extend the %(*)

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 25, 2011

I copied your interpolate method or maybe I missed something?

@pivaldi
Copy link
Contributor Author

pivaldi commented Jul 25, 2011

Hi,
I think that all is right with the commit 808b247 in my branch "interpolation".

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 27, 2011

That looks good.
I thought of something. It would look better if we would use the Object.keys to walk through on the object keys instead of for .. in. Then we wouldn't need to use hasOwnProperty all the time, it would look better.

@pivaldi
Copy link
Contributor Author

pivaldi commented Jul 27, 2011

I'm not sure that was useful because the objects treated haven't prototype but it's done :)

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 28, 2011

Yeash, but I'm using jslint and it always mark it as a warning. :)
Thanks

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 28, 2011

merged

@Ajnasz Ajnasz closed this Jul 28, 2011
@Ajnasz
Copy link
Owner

Ajnasz commented Jul 28, 2011

I also published to npm.

Can I ask one more thing? Could you describe this method in the readme file please?

@pivaldi
Copy link
Contributor Author

pivaldi commented Jul 28, 2011

Done in the branch "master"

@Ajnasz
Copy link
Owner

Ajnasz commented Jul 30, 2011

Thank you very much

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

2 participants