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

Eliminate multiple underscore arguments in tests and examples #54

Closed
jwmerrill opened this issue Jan 25, 2016 · 9 comments · Fixed by #61
Closed

Eliminate multiple underscore arguments in tests and examples #54

jwmerrill opened this issue Jan 25, 2016 · 9 comments · Fixed by #61

Comments

@jwmerrill
Copy link
Contributor

From the mdn page on strict mode

strict mode requires that function parameter names be unique

The house style so far uses underscores for every argument that is not actually used in a semantic action, but this throws errors when run in strict mode. It might be better style to use function (_1, _2, _3) {} (or some other alternative?) instead.

@pdubroy
Copy link
Contributor

pdubroy commented Jan 25, 2016

Yep, you're right. I realized this when I strict-moded a bunch of our core code a while back, but I never got around to finishing the job by fixing the tests and examples.

Other than the tests and examples, do you know of anything else that needs updating?

@pdubroy pdubroy changed the title Multiple underscore arguments are incompatible with "use strict" Eliminate multiple underscore arguments in tests and examples Jan 25, 2016
@jwmerrill
Copy link
Contributor Author

Other than the tests and examples, do you know of anything else that needs updating?

No. I just checked the docs, and they don't seem to use this convention.

@pdubroy
Copy link
Contributor

pdubroy commented Jan 25, 2016

Ok, good to know! I thanks for filing this.

BTW, the style I've settled on is to simply give the unused arguments sensible (if somewhat terse) names. For quotes, braces, etc., I just used open and close, and for keywords I use stuff like ifkw, thenkw, elsekw.

@alexwarth
Copy link
Contributor

Hi Jason and Pat,

I've been using Ohm in my class at UCLA, in strict mode. A style that I've
developed and seems to be working well for us is to name terms that I don't
care about (i.e., that I don't need to use) with names that start with an
underscore, but are more descriptive. E.g., for a rule like this:

IfExpr = if Expr then Expr else Expr

(with rules if, then, and else defined to consume their respective
keywords) I would write the following semantic action:

IfExpr: function(_if, e1, _then, e2, _else, e3) {
...
}

On Mon, Jan 25, 2016 at 8:33 AM, Patrick Dubroy notifications@github.com
wrote:

Yep, you're right. I realized this when I strict-moded a bunch of our core
code a while back, but I never got around to finishing the job by fixing
the tests and examples.

Other than the tests and examples, do you know of anything else that needs
updating?


Reply to this email directly or view it on GitHub
#54 (comment).

@alexwarth
Copy link
Contributor

Seems sensible to me :)

On Mon, Jan 25, 2016 at 9:58 AM, Patrick Dubroy notifications@github.com
wrote:

Ok, good to know! I thanks for filing this.

BTW, the style I've settled on is to simply give the unused arguments
sensible (if somewhat terse) names. For quotes, braces, etc., I just used
open and close, and for keywords I use stuff like ifkw, thenkw, elsekw.


Reply to this email directly or view it on GitHub
#54 (comment).

@pdubroy
Copy link
Contributor

pdubroy commented Feb 14, 2016

Re-opening as we still need to fix some of the tests (e.g. test-ohm-syntax.js).

@alexwarth
Copy link
Contributor

There are still instances of the non-descriptive _ in a bunch of the tests, e.g., in test/test-grammar.js, test/test-semantics.js.

@alexwarth alexwarth reopened this Feb 15, 2016
@pdubroy
Copy link
Contributor

pdubroy commented Feb 15, 2016

I don't think we need to eliminate all use of _. I thought the point was to just to eliminate duplicate parameter names so that they code is compatible with strict mode. I don't see a problem with still using the _ sometimes.

@alexwarth
Copy link
Contributor

Oh, ok. Fine by me!

On Monday, February 15, 2016, Patrick Dubroy notifications@github.com
wrote:

I don't think we need to eliminate all use of _. I thought the point was
to just to eliminate duplicate parameter names so that they code is
compatible with strict mode. I don't see a problem with still using the _
sometimes.


Reply to this email directly or view it on GitHub
#54 (comment).

@pdubroy pdubroy closed this as completed Feb 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants