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

Support end positions for warnings #652

Closed
ybiquitous opened this issue Aug 18, 2022 · 9 comments
Closed

Support end positions for warnings #652

ybiquitous opened this issue Aug 18, 2022 · 9 comments
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed

Comments

@ybiquitous
Copy link
Contributor

Stylelint 14.7.0 has supported end positions for warnings, and the official VSCode extension prepares the end position support.

So, I suggest stylelint-scss also supports end positions.

Currently, stylelint-scss warns for the whole node, but it seems not good DX:

image

Reproduction Details

package.json:

{
  "dependencies": {
    "postcss-scss": "4.0.4",
    "stylelint": "14.10.0",
    "stylelint-codeframe-formatter": "1.1.0",
    "stylelint-scss": "4.3.0"
  },
  "scripts": {
    "test": "stylelint \"*.scss\" --custom-formatter stylelint-codeframe-formatter"
  }
}

.stylelintrc.js:

module.exports = {
  plugins: ["stylelint-scss"],
  customSyntax: "postcss-scss",
  rules: {
    "scss/at-rule-no-unknown": true,
    "scss/at-if-no-null": true,
  },
};

a.scss:

@foo {
  // ...
}

a {
  @if $x == null {}
}

This will change many files (maybe all rules), but I can help from my experience in the stylelint/stylelint repo.

@ybiquitous
Copy link
Contributor Author

For example, we can fix scss/at-rule-no-unknown like this:

diff --git a/src/rules/at-rule-no-unknown/index.js b/src/rules/at-rule-no-unknown/index.js
index ef0634c..58801ac 100644
--- a/src/rules/at-rule-no-unknown/index.js
+++ b/src/rules/at-rule-no-unknown/index.js
@@ -83,8 +83,8 @@ export default function rule(primaryOption, secondaryOptions) {
             ruleName,
             result,
             node: warning.node,
-            line: warning.line,
-            column: warning.column
+            start: { line: warning.line, column: warning.column },
+            end: { line: warning.endLine, column: warning.endColumn },
           });
         }
       }

Then, the output is improved:

image

@kristerkari kristerkari added Enhancement ✨ New feature or request Help wanted 🙋 Help is needed labels Aug 22, 2022
@kristerkari
Copy link
Collaborator

Thanks! This would definitely be a good improvement! :)

@ybiquitous
Copy link
Contributor Author

ybiquitous commented Aug 23, 2022

@kristerkari Thanks for the response. It seems this task needs many PRs, so I created the following list to make progress clear:

I wish this list would help many contributions!

@ybiquitous
Copy link
Contributor Author

I've made one PR #655 to make subsequent PRs easier.

@kristerkari
Copy link
Collaborator

kristerkari commented Dec 13, 2023

@ybiquitous Thanks a ton for the PRs. It really is a lot of work for you to migrate the rules :)

@ybiquitous
Copy link
Contributor Author

Yeah, I've done more than half of the rules for now. I'll complete this issue within a few weeks. 👌🏼

@ybiquitous
Copy link
Contributor Author

ybiquitous commented Dec 18, 2023

I've created Pull Requests for all the rules! See the updated tasklist in #652 (comment).

@kristerkari
Copy link
Collaborator

Thanks again! All merged now. :)

@ybiquitous
Copy link
Contributor Author

Thanks for merging all! I'm closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed
Development

No branches or pull requests

2 participants