Skip to content

To make use files option as string path#40

Open
zemd wants to merge 1 commit intog13013:masterfrom
zemd:patch-1
Open

To make use files option as string path#40
zemd wants to merge 1 commit intog13013:masterfrom
zemd:patch-1

Conversation

@zemd
Copy link
Copy Markdown

@zemd zemd commented Aug 30, 2015

If there is only one file to compile, it is useful to omit array notation

If there is only one file to compile, it is useful to omit array notation
Comment thread index.js
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This can lead to have an array of array !

Change it to: options.files = [].concat(options.files);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

No, it doesn't lead to that.

// input
[].concat([["hello", "world"]])
// output
// [ Array(2) ]
//input
[].concat.apply([], [["hello", "world"]] )
// output
// ["hello", "world"]

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

You are right 👍, but sorry to insist, I prefer to change it to a simpler form, it confused me, so it will do with others :)

This line will be enough: options.files = options.files && [].concat(options.files) || [];

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Your solution also nice, but first you should validate existence of options.files.length to include only array and string.

so it will be:

options.files = options.files && options.files.length && [].concat(options.files) || [];

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Sounds very good for me, go ahead 👍

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

could you make the changes ? or I merge and modify my self ?

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.

2 participants