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

Add problem matchers for deno lint #62

Merged
merged 1 commit into from
Dec 6, 2024

Conversation

r7kamura
Copy link
Contributor

@r7kamura r7kamura commented May 30, 2024

Related to:

I did not add a matcher for deno fmt --check this time, because I thought its output lacked tinformation to be used by problem matchers and could not be handled well.

I have tried this feature on my own sample project and confirmed that it works well like this:

image

The real example is here:

"owner": "deno-lint",
"pattern": [
{
"regexp": "^(?:\\x1B\\[[0-9;]*[a-zA-Z])*(warning|warn|error)(?:\\[(\\S*)\\])?(?:\\x1B\\[[0-9;]*[a-zA-Z])*: (.*?)(?:\\x1B\\[[0-9;]*[a-zA-Z])*$",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

(warning|warn|error)

I know that there is at least an error in the severity of deno lint, but I am not exactly sure what other severities exist. If anyone has exact information, please let me know.

Copy link

@csvn csvn left a comment

Choose a reason for hiding this comment

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

This seems very useful to have included bundled into the denoland/setup-deno action. There's been no activity for half a year now. Any chance for review from the Deno team, @dsherret?

Copy link

Choose a reason for hiding this comment

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

Maybe add deno- as prefix to this file (deno-problem-matchers.json), to make it more consistent with examples from actions/toolkit?

main.js Outdated Show resolved Hide resolved
@r7kamura r7kamura force-pushed the problem-matchers branch 3 times, most recently from 6ec544a to 83b8948 Compare November 27, 2024 13:39
@r7kamura
Copy link
Contributor Author

r7kamura commented Nov 27, 2024

Hmmm, it seems to have stopped working properly now. Perhaps the output format of deno lint has changed since I created this pull request?. I will look into it for a while.


edit: It seems that the output of deno lint is now flexible depending on the number of digits in the line number where the error exists. I've added support for this as well.

I experimented with the modified version and it worked well, so it probably should not be a problem now.

image

@crowlKats crowlKats self-requested a review December 5, 2024 14:10
Copy link
Member

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

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

LGTM, sorry for the delay on this. Will cut a release of setup-deno during the day

@crowlKats crowlKats merged commit 56da422 into denoland:main Dec 6, 2024
27 checks passed
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.

3 participants