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

Generated code style #5

Closed
levithomason opened this issue Mar 1, 2016 · 8 comments
Closed

Generated code style #5

levithomason opened this issue Mar 1, 2016 · 8 comments
Milestone

Comments

@levithomason
Copy link

Currently, create-index files are identified by the string:

'create index';

This string is not linter friendly since it is not an assignment nor call. As far as I know, there is no way to define new directives like 'use strict'.

What about using a comment like eslint, flow type, and others:

/* create-index */

or

/* @create-index */

The latter would give a little more certainty when parsing comments for the flag. A flag like this in general would also allow parsing options following the flag as with eslint for instance.

Thoughts?

@gajus
Copy link
Owner

gajus commented Mar 1, 2016

It should not trigger any linting rule. It is a simple statement. Same as if you were to call a function - you are not expected to assign it to anything. Do you have a specific ESLint rule in mind?

"use strict" is not a directive on itself either. It is a regular statement just like the former. What makes it a directive is that ES engine seeks out for it. Another example is "use strong".

I don't hold strong opinion about this, but introducing a breaking change just for code style reasons is not a good way to maintain a library.

Finally, the only argument that I can think of against this approach is that code minifiers will not remove "create index" statement. But the negligible performance and size implications make this of no difference.

Thoughts?

On Mar 1, 2016, at 22:20, Levi Thomason [email protected] wrote:

Currently, create-index files are identified by the string:

'create index';
This string is not linter friendly since it is not an assignment nor call. As far as know, there is no way to define new directives like 'use strict`.

What about using a comment like eslint, flow type, and others:

/* create-index */
or

/* @create-index */
The latter would give a little more certainty when parsing comments for the flag. A flag like this would also allow parsing options following the flag as with eslint for instance.

Thoughts?


Reply to this email directly or view it on GitHub.

@levithomason
Copy link
Author

Looks like the directive lint rule was a red herring actually. Though I'm realizing linter issues are further reaching than originally thought. Use of semicolons throws in our code bases for instance. We could ignore all index files, but that would exclude manually created index files from linting which is not good either.

Immediate solutions that come to mind are convoluted, but include things like config options or a template file so you could add your own /* eslint-disable */ rules or something.

Ideas on how to have some level of control over what gets generated?

@gajus
Copy link
Owner

gajus commented Mar 1, 2016

Will think about it overnight.

On Mar 1, 2016, at 23:43, Levi Thomason [email protected] wrote:

Looks like the directive lint rule was a red herring actually. Though I'm realizing linter issues are further reaching than originally thought. Use of semicolons throws in our code bases for instance. We could ignore all index files, but that would exclude manually created index files from linting which is not good either.

Immediate solutions that come to mind are convoluted, but include things like config options or a template file so you could add your own /* eslint-disable */ rules or something.

Ideas on how to have some level of control over what gets generated?


Reply to this email directly or view it on GitHub.

@gajus
Copy link
Owner

gajus commented Mar 2, 2016

@levithomason I don't think that configuring output is an option. It makes little sense to be concerned with generated code style. What about adding:

/* eslint-disable */
/* jshint ignore:start */

to the top of generated file?

@levithomason
Copy link
Author

That would solve any potential linter issues and seems like a good iteration to me.

@gajus gajus changed the title Linter friendly file identification Generated code style Mar 2, 2016
@gajus gajus added this to the 1.0.0 milestone Mar 2, 2016
@gajus
Copy link
Owner

gajus commented Mar 2, 2016

Do you still want to create a separate issue for the original issue? The identifier.

@levithomason
Copy link
Author

I think the lint ignore captures the intent.

@gajus gajus closed this as completed in 91c2e4e Nov 1, 2016
@gajus
Copy link
Owner

gajus commented Nov 1, 2016

Use // @create-index to target files.

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

No branches or pull requests

2 participants