Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Enhancing params resolution #25

Merged
merged 1 commit into from
Mar 2, 2016
Merged

Enhancing params resolution #25

merged 1 commit into from
Mar 2, 2016

Conversation

unlucio
Copy link
Contributor

@unlucio unlucio commented Mar 1, 2016

Now you can reference the same parameter multiple times, allowing for more elastic string composition.

Previously, if a param was referenced more than once, only the 1st occurrence was resolved:

/*
{ a: 'Hello :what :what' }
*/
reconfig.get('a', {what: 'world'}) // Hello world :what

With this patch the result will be:

reconfig.get('a', {what: 'world'}) // Hello world world

enabling for more elastic and concise config definitions yielding rather complex results in a simpler way.

would release in version: 2.1.0

@unlucio
Copy link
Contributor Author

unlucio commented Mar 1, 2016

@odino wdyt?

for (var property in parameters) {
value = (value) ? value.replace(':' + property, (parameters[property] || '')) : value;
paramSrc = RegExp(':' + property, 'g');
value = (value) ? value.replace(paramSrc, (parameters[property] || '')) : value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not avoiding creating the paramSrc to begin with?

value = (value) ? value.replace(RegExp(':' + property, 'g'), (parameters[property] || '')) : value;

BTW instead of the ternary operatoor I think it might be more readable to do something like:

if (value) {
  value = value.replace(paramSrc, (parameters[property] || ''));
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason an if inside an if creeps me out :D

Now you can reference the same parameter multiple, allowing for more
elastic string composition.
unlucio added a commit that referenced this pull request Mar 2, 2016
Enhancing params resolution
@unlucio unlucio merged commit 866e7b9 into master Mar 2, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants