Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

[new-rule-option] no-space-keywords for comment-format #4003

Closed
wants to merge 6 commits into from
Closed

[new-rule-option] no-space-keywords for comment-format #4003

wants to merge 6 commits into from

Conversation

Shinigami92
Copy link
Contributor

PR checklist

Overview of change:

You can add a new object in comment-format to add some exceptions for checking special keywords that can ignore the check-space option

e.g.:

{"comment-format": [true, "check-space", {"no-space-keywords": ["TODO", "FIXME"]}]}
// some comment
//TODO: my todo

There is a check that allows an optional colon at the end of the keyword

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

CHANGELOG.md entry:

  • [new-rule-option] no-space-keywords in comment-format

@Shinigami92 Shinigami92 changed the title [WIP] [new-rule-option] no-space-keywords for comment-format [new-rule-option] no-space-keywords for comment-format Jul 28, 2018
@Shinigami92
Copy link
Contributor Author

The formatting in my last push was prettier #4012

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems generally good. Needs documentation on the new option, and I think the hardcoded : should be removed.

src/rules/commentFormatRule.ts Show resolved Hide resolved
src/rules/commentFormatRule.ts Show resolved Hide resolved
src/rules/commentFormatRule.ts Show resolved Hide resolved
@Shinigami92
Copy link
Contributor Author

Will look into the conflict in the next few days

@JoshuaKGoldberg
Copy link
Contributor

Hmm, looks like the changes in this PR are actually already resolved by the ignore-pattern option in #3583. There are some pretty substantial merge conflicts here that are otherwise nontrivial to resolve as well. Closing this PR for housekeeping.

Please comment here or in #3951 if you think that's a mistake and you'd like this PR to be re-opened!

@Shinigami92
Copy link
Contributor Author

I'm with your desicion.

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

Successfully merging this pull request may close these issues.

2 participants