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

feat: Report ranges for diagnostics #358

Merged
merged 11 commits into from
Oct 31, 2023
Merged

feat: Report ranges for diagnostics #358

merged 11 commits into from
Oct 31, 2023

Conversation

adalinesimonian
Copy link
Member

Resolves #336

Reports ranges for diagnostics.

This branch is being used to test stylelint/stylelint#5725.

@ybiquitous
Copy link
Member

ybiquitous commented Apr 20, 2022

Stylelint 14.7.0 now supports ranges!

@ota-meshi
Copy link
Member

ota-meshi commented Jul 7, 2022

I have tried this change with the latest version of Stylelint, but Stylelint may report too wide a range.

We probably need to resolve it issue fix first.
stylelint/stylelint#5694

For example, an error in selector-id-pattern may report the entire CSS rule.

@ybiquitous
Copy link
Member

@ota-meshi stylelint/stylelint#5694 tasks are all done! If you have any questions or troubles, feel free to ask me.

@ota-meshi
Copy link
Member

That's so cool!

@ota-meshi ota-meshi marked this pull request as ready for review August 23, 2022 04:49
@ota-meshi ota-meshi added this to the v1.3.0 milestone Aug 23, 2022
@adrianjost
Copy link

Is there anything missing for getting this into main? Anything I could do to support?

@ntwb ntwb closed this Jan 16, 2023
@ntwb ntwb reopened this Jan 16, 2023
@ntwb
Copy link
Member

ntwb commented Jan 16, 2023

Is there anything missing for getting this into main? Anything I could do to support?

I've added a Release v1.3.0 issues in #437 that I'm hoping resolves tests via linked issue/blockers to then release

As can also be seen, I restarted the tests on this PR and they're also failing and I think related to #426

@alex-page
Copy link

alex-page commented Mar 24, 2023

@ntwb it looks like the checks are green. Are we good to ship this now? Is there anything I can help with?

@jbalsas
Copy link
Contributor

jbalsas commented Mar 28, 2023

Hey @ntwb, we took a quick look to see if we could help push this through the finishing line.

We verified the feature is working as expected locally and pushed this small fix to the feature branch to fix some tests we noticed weren't passing.

Hope that helps. Let us know if there's any way we can help push this through! 🤗

cc @samrose3

@jbalsas
Copy link
Contributor

jbalsas commented Oct 20, 2023

Hey @ntwb, @ota-meshi, just checking to see if there are any short-term expectations for getting this in and a new version of the extension published.

Is there any way we can help get that through the finish line? Happy to chip in as much as we can!

@ota-meshi
Copy link
Member

As far as I know, this PR is awaiting review by team members.

@jbalsas
Copy link
Contributor

jbalsas commented Oct 23, 2023

Thanks so much for getting this back in shape @ota-meshi!! ❤️

I guess there's not much more we can do, right? Let us know if there's any way we can help to see this through!

Copy link
Contributor

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

/me dusts off the stlylint/vscode team member hat that hasn't been worn in a while.

This looks great to me! @ota-meshi thank you for rebasing this to get it into better shape, and thank you @jbalsas for testing this.

I work at the same company as @jbalsas and trust his testing, he's set up an internal fork of the plugin that contains this change and it's working well.

I'd love to see this get merged and released, so we can get avoid needing that internal fork at Shopify!

Copy link
Member

@ybiquitous ybiquitous left a comment

Choose a reason for hiding this comment

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

Thanks a lot. Great work. LGTM! 👍🏼

@ota-meshi ota-meshi merged commit 5ef089f into v1.3 Oct 31, 2023
9 checks passed
@ota-meshi ota-meshi deleted the report-ranges branch October 31, 2023 10:06
@ota-meshi
Copy link
Member

Thank you for reviewing!
I will open a PR that merges the v1.3 branch into main.

ota-meshi added a commit that referenced this pull request Nov 2, 2023
* feat: Allow disposing server, add first LSP test (#326)

* feat: Allow disposing server, add first LSP test

* test: close connection before finishing

* refactor: use stylelint namespace for custom event

* feat: Add restart server command (#339)

Closes #332

* test: add code actions on save end-to-end test (#341)

* test: add code actions on save end-to-end test

* test:test auto-fix on save after problem actions

Quick hack to hopefully stabilize tests on CI for now

* feat: Warn for old Stylelint regardless of lang ID (#340)

Does not warn if Stylelint was resolved globally, only locally.

* test: await functions that may return promises

* fix: de-nest globs used for watching config files (#356)

* fix: de-nest globs used for watching config files

* test: update extension tests

* docs: update changelog

* fix: ts error (#471)

* feat: Report ranges for diagnostics (#358)

* feat: Report ranges for diagnostics

* update snap

* update changelog

* update stylelint

* Fixes server/modules/formatter tests (#450)

Fix copied from original PR

https://github.com/stylelint/vscode-stylelint/pull/426/files#diff-f2680deb1906c0920d4e4b4f73879a32a741ed4288aabb2a218a2eef48f38211

* update snapshot

* update snapshot

* fix

* fix: test cases

---------

Co-authored-by: yosuke ota <[email protected]>
Co-authored-by: Chema Balsas <[email protected]>

---------

Co-authored-by: Adaline Valentina Simonian <[email protected]>
Co-authored-by: Chema Balsas <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: Emit diagnostics for ranges instead of single positions
10 participants