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

Fixes #2663 - No more nested classes #2664

Merged
merged 11 commits into from
May 23, 2023

Conversation

tznind
Copy link
Collaborator

@tznind tznind commented May 22, 2023

Fixes #2663 - Promote all nested classes to full classes

In #2406 many classes were moved from nested to root level. This PR explores moving all classes (except those in SpinnerStyle.cs) out to their own classes.

In some cases e.g. Curses.Window I named the new class CursesWindow which is I think a nice way of grouping things together. See also e.g. Bar (previously BarSeries.Bar) is now BarSeriesBar

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
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working

Copy link
Collaborator

@tig tig left a comment

Choose a reason for hiding this comment

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

Please back out the changes to the ConsoleDrivers and related code. I will take care of this in #2612; it's diverged enough that merging from this PR will be a PITA. Thanks.

@tznind
Copy link
Collaborator Author

tznind commented May 23, 2023

Please back out the changes to the ConsoleDrivers and related code. I will take care of this in #2612; it's diverged enough that merging from this PR will be a PITA. Thanks.

I've reverted everything in the ConsoleDrivers folder to match v2_develop.

I have commented out <WarningsAsErrors>CA1034</WarningsAsErrors> (in the .csproj file) so you can work on the console classes without seeing errors about nesting. But I think it is nice to have as it prevents us creating public nested classes in future (will result in build failure) so if we could turn it back on after the console refactor that would be good.

You can still suppress in source:

#pragma warning disable CA1034 // Nested types should not be visible

[...]

#pragma warning restore CA1034 // Nested types should not be visible

Terminal.Gui/ApplicationRunState.cs Outdated Show resolved Hide resolved
Terminal.Gui/Configuration/Scope.cs Show resolved Hide resolved
Terminal.Gui/Configuration/ScopeJsonConverter.cs Outdated Show resolved Hide resolved
Terminal.Gui/Configuration/ThemeManager.cs Outdated Show resolved Hide resolved
Terminal.Gui/Drawing/LineCanvasCell.cs Outdated Show resolved Hide resolved
@tig tig merged commit e2feeef into gui-cs:v2_develop May 23, 2023
1 check 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.

None yet

3 participants