New: line-comment-position rule (fixes #6077)

[alberto]

What issue does this pull request address?
#6077

What changes did you make? (Give an overview)
Created new rule line-comment-position

Is there anything you'd like reviewers to focus on?

• The regular expression. I modified the original proposal to make some of the patterns only match for exact words, to try to match what JSCS does.
• The documentation. I have been working on and off on this one for a long time, so I may have developed some blindness.
• Should I put the regular expression as is in the docs to indicate the exact default ignore patterns?

[mention-bot]

@alberto, thanks for your PR! By analyzing the annotation information on this pull request, we identified @nzakas, @mysticatea and @gyandeeps to be potential reviewers

[eslintbot]

LGTM

[platinumazure]

If this is constant and can't be overridden, I would lean towards using a regex literal here. Helps avoid Leaning Toothpick Syndrome.

[alberto]

I used a string for symmetry with ignorePattern.

[platinumazure]

Sure, but it's internal. Using a string here results in a performance hit,
and I'm not convinced that it's more readable either.

On Aug 25, 2016 5:31 PM, "alberto" notifications@github.com wrote:

In lib/rules/line-comment-position.js
#6953 (comment):

•                        ignorePattern: {
  

•                            type: "string"
  

•                        },
  

•                        applyDefaultPatterns: {
  

•                            type: "boolean"
  

•                        }
  

•                    },
  

•                    additionalProperties: false
  

•                }
  

•            ]
  

•        }
  

•    ]
  

• },

• create(context) {

•    const DEFAULT_IGNORE_PATTERN = "^\s*(?:eslint|jshint\s+|jslint\s+|istanbul\s+|globals?\s+|exported\s+|jscs|falls?\s?through)";
  

I used a string for symmetry with ignorePattern.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/eslint/eslint/pull/6953/files/55e1849ca32affa2fb639624d14c153cdbd470d0#r76337140,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARWejQEM9RWQjG8h4TAyuPn6uZB_Dh7ks5qjhe-gaJpZM4JpTWN
.

[alberto]

My motivation was that, even if it is internal, the rule source is linked in the docs and could be used as a reference for ignorePattern. As for performance, the RegExp is created once per file, so I'm not sure there is much of a performance hit.

[nzakas]

Tie goes to the author when it's a preference discussion. Let's move on. :)

[platinumazure]

LGTM except two small nitpicks (one empty line that could be collapsed, one suggestion of using regex literal instead of string that will be passed into RegExp constructor).

[nzakas]

This section is supposed to be an introduction to the concept that the rule covers. It helps to show some examples and explain that some style guides prefer one or the other.

[nzakas]

Lgtm, just a note on the docs.

I don't think it's with putting the regular expression in the docs. It's hard to understand as it is, so better to leave in the code.

[eslintbot]

LGTM