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

spaced-comment should allow line ending after markers #12036

Closed
tiansh opened this issue Jul 29, 2019 · 7 comments · Fixed by #12558, basics/vector#111, basics/vector#113, thinkwee/thinkwee.github.io#39 or alxtford/numconv#46
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion bug ESLint is working incorrectly rule Relates to ESLint's core rules

Comments

@tiansh
Copy link

tiansh commented Jul 29, 2019

What rule do you want to change?
spaced-comment

Does this change cause the rule to produce more or fewer warnings?
fewer

How will the change be implemented? (New option, new default behavior, etc.)?
default behavior

Please provide some example code that this change will affect:

/* eslint spaced-comment: ["error", "always", { "line": { "markers": ["/", "#region", "#endregion"] } }] */
/* eslint no-trailing-spaces: ["error"] */

//#region blah
const answer = 42;
//#endregion

console.log(answer);

What does the rule currently do for this code?

6:1 error Expected space or tab after '//#endregion' in comment spaced-comment

using --fix cannot fix it (since there are no possible auto fix)

What will the rule do after it's changed?

pass without tip an error

Are you willing to submit a pull request to implement this change?

Maybe no


spaced-comment does allow line ending after // without using markers. And this behavior should be applied to markers too imo.

@tiansh tiansh added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules triage An ESLint team member will look at this issue soon labels Jul 29, 2019
@platinumazure
Copy link
Member

platinumazure commented Jul 29, 2019

Hi @tiansh, thanks for the issue.

This feels like a bug to me. I think it shouldn't matter if a comment has markers after the content or not.

I'm going to relabel this as a bug, but team members can feel free to correct that if needed.

@platinumazure
Copy link
Member

platinumazure commented Jul 29, 2019

Reproduction here, so marking as accepted.

@platinumazure platinumazure added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly and removed enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jul 29, 2019
@g-plane
Copy link
Member

g-plane commented Jul 29, 2019

@platinumazure Repro link is broken.

@platinumazure
Copy link
Member

@g-plane Sorry, bad paste. It's fixed now.

@mdjermanovic
Copy link
Member

mdjermanovic commented Aug 18, 2019

I'm working on this, but have a question. Should the rule also allow these:

/* eslint spaced-comment: ["error", "always", { "block": { "markers": ["foo"] } }] */

/*foo*/
/* eslint spaced-comment: ["error", "always", { "block": { "markers": ["foo"], balanced: true } }] */

/*foo*/

I.e. ignore a comment that contains only a marker like it already ignores an empty /**/

The issue with //#endregion is that the rule requires a \s after the marker, but there is nothing. Linebreak is not included in comment's value.

// immediately followed by the line ending is allowed simply because it's an empty comment.

@eslint-deprecated eslint-deprecated bot added the auto closed The bot closed this issue label Oct 28, 2019
@eslint-deprecated
Copy link

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that accepted issues failing to be implemented after 90 days tend to
never be implemented, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.

@kaicataldo kaicataldo reopened this Oct 29, 2019
@kaicataldo kaicataldo removed the auto closed The bot closed this issue label Oct 29, 2019
@DmytroLapshyn
Copy link

If anyone cares for a workaround, this worked for me:

"markers": ["#region"],
"exceptions": ["#endregion"]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.