Skip to content

New: directive comments (fixes #260) #320

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

Merged
merged 2 commits into from
Jan 6, 2018
Merged

Conversation

mysticatea
Copy link
Member

Fixes #260.

This PR adds supports of directive comments in <template>:

  • <!-- eslint-disable -->
  • <!-- eslint-enable -->
  • <!-- eslint-disable-line -->
  • <!-- eslint-disable-next-line -->

The mechanism is:

  1. vue/directive-comment rule reports those comments.
  2. The post-processor of this plugin removes vue/directive-comment errors and the reported errors in disabled areas.

This PR adds vue/directive-comment rule to base category, but the rule doesn't report any errors (as more exact, all vue/directive-comment errors are removed in post-process), so it doesn't break user's CI builds.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

Amazing! Really good stuff 👍 Clever approach


```html
<template>
<!-- eslint-disable-next-line vue/max-attributes-per-line -->
Copy link
Member

Choose a reason for hiding this comment

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

I think we should also add this information in README

</template>
```

This rule doesn't make any warning.
Copy link
Member

Choose a reason for hiding this comment

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

doesn't throw

@@ -0,0 +1,24 @@
# support comment-directives in `<template>` (comment-directive)

This rule supports the following comment directives in `<template>`:
Copy link
Member

Choose a reason for hiding this comment

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

Sole purpose of this rule is to provide `eslint-disable` functionality in `<template>`.
It supports usage of the following comments:
...

Plus I think you should add quick information of how this works actually, because it might be a bit confusing for people to see a rule, that actually has nothing in common with any other rule :D

it('disable all rules if <!-- eslint-disable -->', () => {
const code = `
<template>
<!-- eslint-disable -->
Copy link
Member

Choose a reason for hiding this comment

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

Could you perhaps add few more examples with nested/multiline elements like e.g.:

<template>
  <ul>
    <li v-for="item in items" <!-- eslint-disable-line -->
      :class="item.class"
      :title="item.title"
      <!-- eslint-disable-next-line -->
      title="test"
    />
  </ul>
</template>

Copy link
Member Author

@mysticatea mysticatea Jan 6, 2018

Choose a reason for hiding this comment

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

It's just an invalid HTML code. Some no-parsing-error warnings will be shown.

@mysticatea
Copy link
Member Author

Thank you for the review. I updated docs.

Copy link
Member

@michalsnik michalsnik left a comment

Choose a reason for hiding this comment

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

Perfect! Thank you @mysticatea 🥇

@michalsnik michalsnik merged commit ad84e0b into master Jan 6, 2018
@michalsnik michalsnik deleted the comment-directives branch January 6, 2018 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants