Skip to content
This repository has been archived by the owner on Jul 15, 2019. It is now read-only.

A space after a comma in the 'commonLib' string in 'test-descriptor.json' causes a fail #206

Closed
nottoseethesun opened this issue Mar 29, 2014 · 9 comments

Comments

@nottoseethesun
Copy link

commonLib: "foo.js, bar.js", fails where commonLib: "foo.js,bar.js", succeeds.

However, it is common practice to allow a space in a comma-delimited list, for human readability. The fail is a failure to load the files specified.

commonLib: "foo.js, bar.js", should succeed.

@pranavparikh
Copy link

@christopherbalz
Thanks for reporting this. I'll take a look.

@nottoseethesun
Copy link
Author

Welcome - I realized this happens under an old version ( 0.0.84, I believe), so perhaps it's been fixed by now. But if not, it would be really good to fix.

@redonkulus
Copy link

Furthermore @pranavparikh, is there any reason it can't be an array of libraries instead of comma separated string?

@pranavparikh
Copy link

@redonkulus ,
There's no reason for it. I fully support having an array instead of comma separated string ( supporting both though for backward compatibility), wherever there is such case. We'll take it up as enhancement. Thanks for the feedback.

@redonkulus
Copy link

@redonkulus
Copy link

ping @pranavparikh @proverma

@pranavparikh
Copy link

@redonkulus ,

Trimming each element of the array "arrLib" before processing should work. I'll test it out and make the change.

https://github.com/yahoo/arrow/blob/e42b9d49385463ad641c51dc602c638cd59cf923/lib/util/libmanager.js#L30

@pranavparikh
Copy link

#233 should fix this issue

@pranavparikh
Copy link

The fix is out with 0.5.3.
#234 - Created a separate issue ( enhancement) for allowing array for params like commonLib

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants