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

Add rest parameters syntax for multiple params #158

Merged
merged 1 commit into from
Oct 30, 2014

Conversation

juandopazo
Copy link
Member

Hi!

Rest parameter syntax is coming in EcmaScript 6 (already in Firefox Nightly!). It looks like this:

function foo(param1, ...args) {
  // args is an array
}

I think it makes sense to use this syntax instead of YUIDoc's * for multiple parameters. It's easier to remember (I sometimes confuse it with Type[]) and maybe some day we'll be using it in real code. This pull request updates the docs to use ...params, and adds the option for @param {Type} ...params and @param {Type} [...params].

I added tests for this feature but I can't seem to run them from the command line. Is there anything else I should do besides running node tests\parser.js?

@juandopazo
Copy link
Member Author

npm test. Obviously.

It doesn't work in my PC though. Maybe a Windows vs Linux thing.

@juandopazo
Copy link
Member Author

I'm ok with a "no". I just wanted to discuss it with working code.

@okuryu
Copy link
Member

okuryu commented May 9, 2013

Looks good to me. Do not need to leave to document the old format?

@okuryu
Copy link
Member

okuryu commented Oct 27, 2014

I'd like to merge this. I just confirmed that the conflicts were resolved and passed tests on my box.

/cc @caridy

@caridy
Copy link
Member

caridy commented Oct 27, 2014

LGTM. Does this maintain BC? It looks like it does. Then I'm good with it.

@okuryu
Copy link
Member

okuryu commented Oct 27, 2014

Yup, the user guide will suggests new syntax, but the parser maintains backward compatibility.

@juandopazo
Copy link
Member Author

\o/

@okuryu
Copy link
Member

okuryu commented Oct 28, 2014

I'm planning the next release v0.4.0, so this feature will be includes into that.

@okuryu okuryu self-assigned this Oct 29, 2014
@yahoocla
Copy link

CLA is valid!

@juandopazo
Copy link
Member Author

@okuryu I resolved the conflicts!

@okuryu okuryu merged commit 7a1647c into yui:master Oct 30, 2014
@okuryu
Copy link
Member

okuryu commented Oct 30, 2014

@juandopazo Nice! I just merged, thanks!

@okuryu okuryu mentioned this pull request Nov 10, 2014
17 tasks
@okuryu okuryu added this to the v0.4.0 milestone Nov 28, 2014
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.

4 participants