Skip to content

feat(diagnostics): line comments#4

Merged
andy9a9 merged 3 commits intomainfrom
feat/comments_warns
Mar 21, 2026
Merged

feat(diagnostics): line comments#4
andy9a9 merged 3 commits intomainfrom
feat/comments_warns

Conversation

@andy9a9
Copy link
Copy Markdown
Owner

@andy9a9 andy9a9 commented Jan 14, 2026

  • devicetree.enableWarnings -> devicetree.diagnostics.enableWarnings
    • move enableWarnings under diagnostics
  • add option to exclude comments from line length diagnostics
    • this will be applied only if warnings are enabled

@andy9a9 andy9a9 requested a review from mkessler001 January 14, 2026 18:44
Copy link
Copy Markdown
Collaborator

@mkessler001 mkessler001 left a comment

Choose a reason for hiding this comment

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

I added a small addon that re-analyzes open files when the settings change (e.g. line length is modified).
There is another commit marked as TODO, which comes with two test cases, which are currently failing (block comment, mistreated strings as comments).

Comment thread src/diagnostics.ts
@andy9a9
Copy link
Copy Markdown
Owner Author

andy9a9 commented Jan 29, 2026

I added a small addon that re-analyzes open files when the settings change (e.g. line length is modified).

I was thinking about the same direction, but didn't watch to add this feature. Maybe I thought in a wrong way, but once the file is formatted and you change the settings it could lead into wrong line lengths. If you add the "watching" feature just for the line length then it could be little bit inconsistent.

Overall, what's the benefit of having this feature?

@mkessler001
Copy link
Copy Markdown
Collaborator

I added a small addon that re-analyzes open files when the settings change (e.g. line length is modified).

I was thinking about the same direction, but didn't watch to add this feature. Maybe I thought in a wrong way, but once the file is formatted and you change the settings it could lead into wrong line lengths. If you add the "watching" feature just for the line length then it could be little bit inconsistent.

Overall, what's the benefit of having this feature?

I see a problem when users change settings. Without the update mechanism, they would need to restart VSCode (or at least reload the extensions) to apply the new settings. This is not an intuitive behavior.
I'm not sure if I understand your argument. When the file is formatted and you change the settings (e.g. reduce line length from 120 to 80), I would expect to see warnings for lines longer than 80 characters.

@andy9a9 andy9a9 force-pushed the feat/comments_warns branch from c869f94 to ef6e925 Compare February 6, 2026 13:08
@andy9a9 andy9a9 force-pushed the feat/comments_warns branch from ef6e925 to a4e5850 Compare March 18, 2026 18:23
@andy9a9 andy9a9 requested a review from mkessler001 March 18, 2026 18:24
@andy9a9 andy9a9 force-pushed the feat/comments_warns branch from a4e5850 to 84edb81 Compare March 18, 2026 18:30
Copy link
Copy Markdown
Collaborator

@mkessler001 mkessler001 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just keep in mind that this is a breaking change since we moved enableWarnings to diagnostics.enableWarnings.

@andy9a9 andy9a9 merged commit f63c590 into main Mar 21, 2026
5 checks passed
@andy9a9 andy9a9 deleted the feat/comments_warns branch March 21, 2026 19:07
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.

2 participants