Skip to content

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented Jul 1, 2025

Fixes

Proposed Changes/Todos

  • Add Terminal.Gui.Analyzers project (and tests).
  • Add first diagnostic: warn if event handler for Accepting does not set e.Handled = true.

analyzer

Documentation

Note for the documentation, there is an established format:

https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md

This format is actually enforced by compiler warnings and mismatches with diagnostic rules are detected. For this reason it makes sense to follow established best practice rather than have docs in our main docs dir (i.e. docfx).

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings (actually it generates new ones! that's the point)
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

@tznind tznind marked this pull request as ready for review July 5, 2025 01:35
@tznind tznind requested a review from tig as a code owner July 5, 2025 01:35
@tznind
Copy link
Collaborator Author

tznind commented Jul 5, 2025

Ok this is ready for review when you have a chance.

I have not fixed the missing e.Handled in this PR as I would rather do that as a followup.

Followup actions once merged

  • Confirm that the analyzer properly ships in nupkg (I've tested pack locally and can see analyzer dll so should work)
  • Update help uri for rules to point to main gui.cs repo instead of my branch
  • Mark the new rule as Shipped in the markdown
  • Update existing code to address the warning throughout the codebase.

Next rule I want to add is to check for missing Shutdown i.e. if you call Init in a method you typically should also call Shutdown nearby.

@tig tig merged commit 3a64519 into gui-cs:v2_develop Jul 7, 2025
11 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.

Button.IsDefault is confusing

2 participants