190-strange-coloring-ega-mode #233
Conversation
Co-authored-by: <[email protected]>
Co-authored-by: <[email protected]>
Co-authored-by: <[email protected]>
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive enhancement to the Consolonia framework's color management and console initialization process. The changes primarily focus on implementing a more flexible and dynamic color mode system for console applications. A new The modifications span multiple components, including the application startup process, color extension classes, and test infrastructure. The changes enable more sophisticated color handling, allowing developers to specify different color modes during application initialization. The implementation includes support for various color modes like Colored, Transparent, and Shaded, with methods to convert between console and Avalonia color representations. The refactoring also involves updates to theme handling, test organization, and color resource management, providing a more robust and flexible approach to console color management in the Consolonia framework. Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (25)
src/Consolonia.Themes/Templates/Controls/Popup.axaml (1)
10-10: Remove extra blank line for clarityThe extra blank line here appears unused. Removing it keeps the XAML concise and easier to read.
src/Consolonia.Core/Drawing/BrushExtensions.cs (4)
11-11: Opacity TODO
Brush opacity is unhandled. Marked as a TODO—ensure it’s addressed before final release.
63-63: Radial gradient color
Using distance-based interpolation for radial gradients is logical. Consider edge cases if radius is 0.
81-81: Conic gradient average
Blends horizontal and vertical interpolation, but keep an eye on gradient accuracy for more complex shapes.
88-89: Fallback for unsupported brushes
RaiseNotSupportedthen returning a default color. Possibly consider a fallback color or reconsider the exception scenario.src/Consolonia.Core/Drawing/PixelBufferImplementation/Pixel.cs (1)
107-139: Refine alpha-based conditional checks & consider factoring out complex conditions.
The new blending logic appears to handle alpha values by selectively merging foreground and background colors. The direct comparison to0x0and0xFFmay overlook any semi-transparency scenarios between these extremes. Additionally, factoring out these branching conditions into well-named helper methods could enhance readability.- if (pixelAbove.Background.Color.A == 0x0) - { - ... - } - else - { - if (!pixelAbove.Foreground.NothingToDraw()) - { - ... - } - else - { - if (pixelAbove.Background.Color.A == 0xFF) - ... - else - { - ... - } - } - } + // Example approach: + if (IsFullyTransparent(pixelAbove.Background.Color)) + { + // ... + } + else + { + if (!pixelAbove.Foreground.NothingToDraw()) + { + // ... + } + else + { + if (IsFullyOpaque(pixelAbove.Background.Color)) + // ... + else + // ... + } + }src/Consolonia.Core/Drawing/PixelBufferImplementation/IConsoleColorMode.cs (1)
1-10: Consider adding XML doc comments.
The interface is concise and readable. However, documenting the expected behavior ofBlendandSetAttributeswould help ensure consistent implementations and usage across different console color modes.src/Consolonia.Gallery/App.cs (1)
20-23: Address the TODO for 16-color console fallback.
Adding theFluentThemeis straightforward, but the comment indicates a plan to switch automatically if only 16 colors are available. Incorporating a runtime check (or fallback logic viaIConsoleColorMode) can ensure the user experience remains consistent on limited-color environments.Do you want me to propose a fallback implementation that checks for 16-color support?
src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColor.cs (1)
1-38: Well-structured markup extension – consider usage examples.
This extension offers a neat way to transformConsoleColorinto Avalonia colors. Consider including an XML comment or usage example showing how to applyEgaColorExtensionin XAML.src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs (1)
46-47: Remove unused TODO comments if no longer relevant.
Lines 46 and 47 include "TODO" placeholders for freeing unmanaged resources. If there are no unmanaged resources, consider removing these to maintain code cleanliness.src/Consolonia.Core/Drawing/PixelBufferImplementation/RgbConsoleColorMode.cs (2)
13-22: Improve function documentation for clarity.
The inline comments mention “by chatGPT” and detail alpha compositing. Consider adding doc comments or clarifying references. The reference to an AI-based snippet may not be necessary in production code.-// by chatGPT o1 // Convert alpha from [0..255] to [0..1]
38-55: Consider user-defined brightness or shading factors.
The approach inSetAttributesusesBrightenorShadebased on the weight, which is a thoughtful design. For more fine-grained control, consider allowing user-defined brightness/shading factors or adjusting them based on weight thresholds to handle a wider range of font weights.src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs (1)
33-59: Potentially extend terminal detection logic.
The switch onTERMcovers many common values but may miss other color-capable terminals. If broad user environments are expected, consider adding more granular detection or providing a fallback to a 24-bit color mode if uncertain.src/Consolonia.Gallery/View/ControlsListView.axaml (1)
35-37: Revisit combo box ordering if additional themes are added.
The reordering places "Fluent" first and "Material" later. If themes grow in number, consider using data binding for dynamic theme loading for maintainability.src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBuffer.cs (1)
30-30: Consider making the default fill color configurable.Although initializing each pixel with a black background is fine in many cases, offering a way to configure the default color (through constructor parameters or application settings) may make the codebase more flexible, particularly for theming or different console color modes.
src/Consolonia.Themes/Templates/Controls/Helpers/ConsoloniaTextPresenter.cs (2)
59-60: Allow more flexible caret brush options.Currently, the code enforces a strictly transparent caret brush. While this might be the desired behavior, consider providing a controlled fallback or user-configurable approach, rather than throwing an exception, to handle partial transparency or alternative caret visuals.
79-79: Enable future color customization for the caret.Forcing the caret to be transparent ensures manual drawing, but might limit future color customizations. Consider exposing a property or mechanism to select from predefined caret colors or brushes if the design requirements expand.
src/Consolonia.Gallery/View/ControlsListView.axaml.cs (1)
111-111: Check for style index safety.Overwriting
Application.Current.Styles[0]can be fragile if other parts of the app rely on a specific style ordering. Consider appending or replacing styles at a more controlled index or adding a dedicated style slot for dynamic theme swaps.src/Example/Views/DataGridTestWindow.axaml (1)
47-48: Use a dynamic resource for theming consistencyReplacing the
Foregroundproperty with a solid "Gray" color looks straightforward. However, consider using aDynamicResource(e.g.,ThemeForegroundBrush) to stay consistent with the theming approach used in the rest of the codebase. This ensures that if the theme changes, the foreground color gets updated accordingly.src/Consolonia.Core/Infrastructure/InputLessDefaultNetConsole.cs (1)
72-73: Dependency injection refactor considerationRetrieving
IConsoleColorModeat runtime viaAvaloniaLocatorworks but may reduce testability. Investigate whether injectingIConsoleColorModethrough the constructor or a property would simplify mocking and improve maintainability.src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs (2)
57-76: LeverageFontWeight? weightinSetAttributes.
Theweightparameter is declared but not applied in the current console drawing logic. If bold text is ultimately supported, consider applying it here or remove it if it’s not used.
136-157: Consider a fallback step for partial shading.
When shading multiple times, many colors eventually map down toConsoleColor.Black. If you want finer gradations, you might consider intermediate mappings rather than jumping directly to black.src/Consolonia.Core/Drawing/DrawingContextImpl.cs (3)
530-530: Generating background color from brush in FillRectangle: Good approach.You’re now calling
brush.FromPosition(x, y, ...). Ensure any possible performance overhead of repeated calls is acceptable for large draw areas.
730-730: Rotation check: Consider supporting transformations in a future iteration.Currently, only translation is allowed. If advanced transformations become a requirement, revisit this area.
743-743: Refactor to retrieve color once: Minor style improvement.You retrieve
solidColorBrush.ColorintoforegroundColor. This is fine, but if we expand logic, consider caching or verifying alpha within the same block.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
src/.editorconfig(1 hunks)src/Consolonia.Core/ApplicationStartup.cs(4 hunks)src/Consolonia.Core/Controls/Dialog/DialogWrap.axaml(1 hunks)src/Consolonia.Core/Drawing/BrushExtensions.cs(4 hunks)src/Consolonia.Core/Drawing/DrawingContextImpl.cs(5 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/DrawingBoxSymbol.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColor.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/IConsoleColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/ISymbol.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/Pixel.cs(4 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackground.cs(2 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs(0 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBuffer.cs(2 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelForeground.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/RgbConsoleColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/SimpleSymbol.cs(1 hunks)src/Consolonia.Core/Drawing/RenderTarget.cs(1 hunks)src/Consolonia.Core/Infrastructure/InputLessDefaultNetConsole.cs(2 hunks)src/Consolonia.Gallery/App.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml(3 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml.cs(5 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryDataGrid.axaml.cs(1 hunks)src/Consolonia.Gallery/Program.cs(1 hunks)src/Consolonia.Gallery/View/ControlsListView.axaml(1 hunks)src/Consolonia.Gallery/View/ControlsListView.axaml.cs(1 hunks)src/Consolonia.NUnit/ConsoloniaAppTestBase.cs(1 hunks)src/Consolonia.NUnit/UnitTestConsole.cs(1 hunks)src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs(2 hunks)src/Consolonia.Themes/Fluent/FluentColors.axaml(1 hunks)src/Consolonia.Themes/Material/MaterialColors.axaml(1 hunks)src/Consolonia.Themes/Templates/Controls/CalendarItem.axaml(3 hunks)src/Consolonia.Themes/Templates/Controls/DataGrid.axaml(1 hunks)src/Consolonia.Themes/Templates/Controls/DialogWindow.axaml(1 hunks)src/Consolonia.Themes/Templates/Controls/Helpers/ConsoloniaTextPresenter.cs(2 hunks)src/Consolonia.Themes/Templates/Controls/Popup.axaml(1 hunks)src/Consolonia.Themes/Templates/TurboVisionDefaultColors.axaml(1 hunks)src/Consolonia.Themes/TurboVisionBlack/TurboVisionBlackColors.axaml(1 hunks)src/Consolonia.Themes/TurboVisionDark/TurboVisionDarkColors.axaml(1 hunks)src/Example/Views/DataGridTestWindow.axaml(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/Assembly.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/ColorTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingBoxSymbolTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs(5 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/GlyphTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/MiscTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelBackgroundTests.cs(4 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelBufferTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelForegroundTests.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelTests.cs(3 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/SimpleSymbolTests.cs(2 hunks)
💤 Files with no reviewable changes (1)
- src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs
✅ Files skipped from review due to trivial changes (6)
- src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColorMode.cs
- src/Consolonia.Gallery/Program.cs
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/MiscTests.cs
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelForegroundTests.cs
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/ColorTests.cs
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingBoxSymbolTests.cs
🧰 Additional context used
📓 Learnings (3)
src/Consolonia.Themes/TurboVisionBlack/TurboVisionBlackColors.axaml (1)
Learnt from: tomlm
PR: jinek/Consolonia#122
File: src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml:5-28
Timestamp: 2024-11-12T04:11:38.190Z
Learning: In the project, using `Mode="Shaded"` in `ConsoleBrush` within theme files is intentional and does not cause runtime issues.
src/Consolonia.Themes/Templates/TurboVisionDefaultColors.axaml (1)
Learnt from: tomlm
PR: jinek/Consolonia#122
File: src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml:5-28
Timestamp: 2024-11-12T04:11:38.190Z
Learning: In the project, using `Mode="Shaded"` in `ConsoleBrush` within theme files is intentional and does not cause runtime issues.
src/Consolonia.Themes/TurboVisionDark/TurboVisionDarkColors.axaml (1)
Learnt from: tomlm
PR: jinek/Consolonia#122
File: src/Consolonia.Themes.TurboVision/Themes/TurboVisionDark/TurboVisionDarkColors.axaml:5-28
Timestamp: 2024-11-12T04:11:38.190Z
Learning: In the project, using `Mode="Shaded"` in `ConsoleBrush` within theme files is intentional and does not cause runtime issues.
🔇 Additional comments (101)
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelBufferTests.cs (1)
6-6: Namespace restructuring looks good.
The namespace change aligns with the broader refactoring initiative to group tests under WithLifetimeFixture. This helps maintain clearer organization and categorization of lifetime-related tests.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/GlyphTests.cs (1)
4-4: Namespace name alignment looks good.
Renaming the namespace to include WithLifetimeFixture clarifies the test context and follows a logical naming convention.
src/Consolonia.Themes/Templates/Controls/Popup.axaml (2)
2-2: Removal of drawing namespace is consistent with the refactor
Removing the xmlns:drawing (along with any references to ConsoleBrush) declutters the XAML and aligns with the shift toward using standard dynamic resources. This change appears correct and follows the broader refactor across the project.
9-9: Use of ThemeTransparentBrush for the Panel background
Switching the Panel’s background to a DynamicResource named ThemeTransparentBrush provides a more unified approach to theme-based styling. This is a good step toward maintaining consistent theming across the application.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/Assembly.cs (1)
10-10: Confirm that all references to the old namespace are updated
Changing the namespace to Consolonia.Core.Tests.WithLifetimeFixture is a sensible reorganization for test-related fixtures. However, please ensure that all references (e.g., using statements, test runners, etc.) still align to the new namespace so that no test or build failures are introduced.
Run the following script to locate references to the original namespace across the codebase:
✅ Verification successful
No namespace reference issues found
Based on the search results, all references to Consolonia.Core.Tests are correctly used in the codebase:
- All test files in the
WithLifetimeFixturedirectory consistently use the namespaceConsolonia.Core.Tests.WithLifetimeFixture - The
InternalsVisibleToattributes in various assembly files correctly reference the test project asConsolonia.Core.Tests - The solution file correctly references the test project as
Consolonia.Core.Tests
There are no references to any old or incorrect namespace that would need updating.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching for the old namespace references
rg -A 5 "Consolonia.Core.Tests\W"
Length of output: 8533
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelTests.cs (5)
7-7: Namespace migration
The updated namespace location under WithLifetimeFixture is clear and consistent. No issues found.
146-149: Ensure color parsing aligns with intended shading logic
The change from a PixelBackgroundMode to a semi-transparent color (Color.Parse("#7F000000")) is more flexible for shading. Please confirm that the blending logic properly accounts for alpha values across all supported rendering scenarios.
164-175: Verify shading behavior with darker backgrounds
Using a semi-transparent black overlay is a valid approach. However, ensure that blending with already dark backgrounds does not inadvertently produce overly dark or fully opaque visuals. Consider verifying the numeric results of the blended color in unit tests for stronger reliability.
177-186: Good addition: Checking text visibility under semi-transparent backgrounds
This test effectively verifies that text rendered beneath a partially transparent overlay remains legible. Good job expanding coverage in this scenario.
187-195: Coverage expansion for text rendering with null character
Adding a test to ensure the text remains visible when blended with a char.MinValue symbol is particularly useful. This ensures edge cases around zero-width or special characters are handled gracefully.
src/Consolonia.Core/Drawing/PixelBufferImplementation/ISymbol.cs (1)
18-18: Renaming method for clarity
Renaming IsWhiteSpace() to NothingToDraw() provides a more descriptive semantic for indicating the absence of content to render. This helps unify the concept across various symbol implementations.
src/Consolonia.Core/Drawing/PixelBufferImplementation/SimpleSymbol.cs (2)
48-48: Method rename aligns with interface updates
The NothingToDraw() method name neatly aligns with the recent ISymbol interface change from IsWhiteSpace(), reflecting a clearer usage intent for detecting empty or non-drawable content.
55-55: Conditional blending logic
By returning this when symbolAbove.NothingToDraw() is true, the method ensures that a non-drawable symbol above doesn't overwrite the current symbol. This logic is intuitive and preserves layering semantics.
src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelForeground.cs (1)
99-103: New method for pixel-level emptiness check
Introducing NothingToDraw() here adds a consistent, centralized way to determine if the foreground is effectively non-renderable (transparent color or a symbol that has nothing to display). This neatly complements the rest of the code’s approach.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/SimpleSymbolTests.cs (1)
109-111: Test coverage for 'NothingToDraw()'
The test now asserts that an empty string is considered non-drawable, whereas a space is treated as drawable content. This matches the new semantic distinction and confirms the intended behavior.
src/Consolonia.Core/Drawing/PixelBufferImplementation/DrawingBoxSymbol.cs (3)
34-34: Simplify property accessor.
Changing Width from a backing field to a computed property returning 1 is clean and consistent with the static nature of the width for box symbols.
36-38: Renamed method for clarity.
Renaming IsWhiteSpace() to NothingToDraw() more accurately reflects its purpose and aligns with the interface rename. This helps improve code readability.
43-43: Keep usage consistent with renamed method.
Using NothingToDraw() within Blend maintains consistency with the new naming convention and clarifies when no drawing is required.
src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackground.cs (6)
9-9: DebuggerDisplay attribute helps with debugging clarity
This annotation is concise and clear.
10-10: Leveraging readonly record struct
Using a C# record struct for immutability and value semantics is a good choice here.
12-12: Default constructor clarification
This constructor defaults to Colors.Transparent. Make sure it aligns with your intended usage scenarios for an uninitialized background.
18-18: Public property initialization
Initializing the Color property with = color succinctly captures the constructor parameter.
22-22: Equality check relies solely on Color
Only comparing Color is fine if background mode was intentionally removed.
32-32: Hash code generation
Combining just Color is in line with the updated equality logic.
src/Consolonia.Themes/Templates/TurboVisionDefaultColors.axaml (14)
4-4: Namespace alias corrected
Using xmlns:consolonia="..." instead of xmlns:drawing="..." is consistent with the new approach.
7-8: ThemeForegroundBrush
Switches from a custom brush type to <SolidColorBrush>, referencing an EGA color. Looks good.
9-10: ThemeBackgroundBrush
Defines a Gray background using EGA color. Straightforward.
11-12: ThemeChooserBackgroundBrush
Uses Cyan from EGA color. Approved for simpler color usage.
13-14: ThemeActionBackgroundBrush
Uses Green from EGA color for an action or highlight background.
15-16: ThemeSelectionBackgroundBrush
DarkBlue background definition is consistent.
17-18: ThemeSelectionForegroundBrush
Foreground set to White. No issues identified.
19-20: ThemeAlternativeBackgroundBrush
Uses White color as an alternative background.
21-22: ThemeBorderBrush
DarkGray color assigned for borders. Fine usage.
23-24: ThemeNoDisturbBrush
Reused DarkGray color. Matches existing palette logic.
25-26: ThemeShadeBrush
Color="{consolonia:EgaColor Mode=Shaded}". Verify that the shading logic is supported in your new EgaColor approach.
27-28: ThemePseudoShadeBrush
Uses basic Black color. Seems consistent with theming approach.
29-30: ThemeErrorBrush
Red color for error states is standard.
31-32: ThemeTransparentBrush
Transparent mode usage. Confirm it is processed correctly in the new color pipeline.
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml.cs (7)
4-4: Added System.Linq
Including LINQ is essential for later enumerations.
8-8: Using EgaConsoleColor
Importing EgaConsoleColor fits the updated color mode approach.
20-21: Binding to FourBitControl
FourBitControl.ItemsSource = ConsoleColorItem.GetAll() elegantly populates the control with console color items.
55-55: Changed from ConsoleBrush to IBrush
Using a more general interface broadens brush compatibility in Avalonia.
65-65: Creating SolidColorBrush
Replacing the old ConsoleBrush references with SolidColorBrush is consistent with your new design.
69-69: IBrush property
Renamed property ensures alignment with Avalonia’s standard IBrush.
90-107: New ConsoleColorItem class
Provides a straightforward mapping from ConsoleColor to Avalonia brush. It's a clean approach for color enumerations.
src/Consolonia.Core/ApplicationStartup.cs (9)
6-7: Imports for new color mode
References to PixelBufferImplementation and EgaConsoleColor confirm the updated console color approach.
20-20: Overloaded StartConsolonia
Automatically uses new EgaConsoleColorMode(). Good default.
23-23: Extended StartConsolonia signature
Offers user-defined IConsole and IConsoleColorMode. Increases flexibility.
25-25: BuildLifetime usage
Brings console and color mode together for the final lifetime.
32-32: UseStandardConsole
Chaining .UseConsoleColorMode(new EgaConsoleColorMode()) ensures a default color mode is set.
40-43: UseConsoleColorMode extension
Allowing the app builder to attach a color mode is a good modular approach.
59-59: BuildLifetime signature expanded
Accepting IConsoleColorMode highlights the new color pipeline from the start.
65-65: Color mode integrated into the builder
Calls .UseConsoleColorMode(consoleColorMode), unifying pipeline configuration.
79-79: Spacing or placeholder
Just a line break—no action needed.
src/Consolonia.Core/Drawing/BrushExtensions.cs (4)
7-7: Refactored from ConsoleBrush to static BrushExtensions
Naming is aligned with typical extension patterns.
9-9: New FromPosition method
Central entry point for extracting a color from a brush at a given coordinate.
36-36: Returning color for linear gradient
Blending horizontal and vertical stops is an interesting approach. Validate visual correctness.
84-86: Simplified ISolidColorBrush
Directly returning solidColorBrush.Color is a clean improvement.
src/Consolonia.Core/Drawing/PixelBufferImplementation/Pixel.cs (3)
4-4: No concerns on the new imports.
The addition of Avalonia and Avalonia.Media references is consistent with the updated color-handling approach.
44-44: Confirm default background logic.
Removing PixelBackgroundMode.Transparent in favor of new PixelBackground() is logical. However, please verify that the default state of PixelBackground fully meets your intended transparency or coloring requirements in all usage contexts.
153-154: Ensure the service locator is populated with IConsoleColorMode.
The call to AvaloniaLocator.Current.GetRequiredService<IConsoleColorMode>() will throw an exception if the service is not registered. Please confirm that your startup configuration always provides an IConsoleColorMode instance.
✅ Verification successful
Let me gather more information about the registration of the service.
Let me try a different approach to find the service registration.
Let me check how this service is used in the application startup.
The IConsoleColorMode service is properly registered during application startup
The service is always registered through one of these paths:
- Default registration via
StartConsolonia<TApp>()which usesEgaConsoleColorMode - Custom registration via
StartConsolonia<TApp>(IConsole, IConsoleColorMode, string[]) - Platform-specific registration via
PlatformSupportExtensionswhich chooses betweenRgbConsoleColorMode(design mode) orEgaConsoleColorMode(runtime)
The service locator call in Pixel.cs is safe as the service is guaranteed to be registered before any UI rendering occurs.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find code paths that register IConsoleColorMode to ensure it is always registered before usage.
rg -A 5 "IConsoleColorMode"
Length of output: 5724
Script:
#!/bin/bash
# Find where the service is registered with AvaloniaLocator
ast-grep --pattern 'With($consoleColorMode)'
Length of output: 46
Script:
#!/bin/bash
# Look for AppBuilder registration patterns
rg -A 3 "builder\.With\("
Length of output: 667
Script:
#!/bin/bash
# Look for the usage of UseConsoleColorMode in the startup code
rg -B 5 -A 5 "UseConsoleColorMode|StartConsolonia"
Length of output: 4271
src/Consolonia.Core/Controls/Dialog/DialogWrap.axaml (1)
6-6: DynamicResource usage looks good.
Switching from a static brush to a dynamic resource for the ThemeShadeBrush helps with theming. Ensure that fallback values or error-handling is considered in case the theme resource is missing.
src/Consolonia.Themes/TurboVisionDark/TurboVisionDarkColors.axaml (1)
6-29: Confirm removal of shaded functionality
Replacing all ConsoleBrush usages (with Mode="Shaded") by SolidColorBrush potentially drops the console shading logic. If shading is still desired, consider providing an alternative implementation or verifying that the new solid colors meet the design requirements.
src/Consolonia.Themes/TurboVisionBlack/TurboVisionBlackColors.axaml (1)
6-29: Ensure design consistency with updated brush types
Shifting away from ConsoleBrush to SolidColorBrush might introduce visual or functional differences, particularly if the original mode-based shading was important. Verify that using plain solid colors matches the intended UI scheme.
src/Consolonia.Themes/Material/MaterialColors.axaml (1)
7-41: Validate transformed color resources
All brush definitions have changed from ConsoleBrush to SolidColorBrush. Double-check that the resulting colors and opacity levels (e.g., #7F757575) accurately replicate the desired visual shading.
src/Consolonia.Themes/Fluent/FluentColors.axaml (1)
7-41: Review potential impact on Fluent color design
Removing ConsoleBrush and introducing SolidColorBrush for theme elements might affect how translucency or shading was originally handled. Ensure that transitioning to SolidColorBrush with #7F767676 still aligns with the planned Fluent aesthetics.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs (1)
18-20: Verify the parameters passed to BuildLifetime.
An empty bracket [] is passed as an additional parameter list, which might be a placeholder. Ensure this is the intended usage or consider removing it for clarity if no extra parameters are needed.
-lifetime = ApplicationStartup.BuildLifetime<ContextApp2>(new DummyConsole(), new RgbConsoleColorMode(), []);
+// If no additional parameters are required, you may remove the empty array:
+lifetime = ApplicationStartup.BuildLifetime<ContextApp2>(new DummyConsole(), new RgbConsoleColorMode());✅ Verification successful
Let me verify the usage of BuildLifetime method by examining its implementation and other usages in the codebase.
Let me check the implementation of BuildLifetime to understand its parameter requirements.
The empty array parameter is correct and required
The BuildLifetime method's signature explicitly requires a string[] args parameter, which represents command-line arguments. The empty array [] is a valid C# syntax for creating an empty string array when no command-line arguments are needed for the test. This is consistent with other usages in the codebase, such as in ConsoloniaAppTestBase.cs where Args is passed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the implementation of BuildLifetime method
ast-grep --pattern 'BuildLifetime<$_>($_,$_,$_)'
# Search for other usages of BuildLifetime
rg "BuildLifetime" -A 2
Length of output: 1553
Script:
#!/bin/bash
# Get the implementation of BuildLifetime method
rg -A 15 "public static ClassicDesktopStyleApplicationLifetime BuildLifetime<TApp>" src/Consolonia.Core/ApplicationStartup.cs
Length of output: 827
src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs (1)
30-30: Chained method call for better readability.
.UseConsole(console).UseAutoDetectConsoleColorMode() is succinct. However, if additional configuration is added in the future, splitting this call might improve clarity. For now, this is acceptable.
src/Consolonia.Gallery/View/ControlsListView.axaml (1)
33-33: Default selection alignment.
Setting SelectedIndex="0" ensures "Fluent" is chosen by default, matching the IsSelected="True" attribute on the corresponding ComboBoxItem. This is consistent and provides a clear default.
src/Consolonia.NUnit/ConsoloniaAppTestBase.cs (1)
48-48: Ensure color mode consistency across the application.
Passing a specific color mode (new RgbConsoleColorMode()) to build the application lifetime might break existing tests or usage patterns if other tests or components rely on a different color mode. Verify that this shift aligns with the intended color behavior throughout the application.
src/Consolonia.Themes/Templates/Controls/DialogWindow.axaml (1)
28-31: Great move towards consistent theming via dynamic resources
Switching from ConsoleBrush to dynamic resources (ThemeShadeBrush and ThemeTransparentBrush) unifies style handling and ensures that theme updates can be applied at runtime. This aligns well with the overall approach of flexible theming seen throughout the PR.
src/Consolonia.Themes/Templates/Controls/CalendarItem.axaml (3)
3-3: Namespace cleanup confirmed
Removing the unused drawing namespace reference reduces clutter and reflects the project's shift away from ConsoleBrush. This is a clear improvement in maintainability.
24-24: Unified transparent background resource
Using ThemeTransparentBrush across the calendar headers is consistent with the updated theming approach. This harmonizes transparency styling with the rest of the codebase.
35-35: Pressed state consistency
Replacing the pressed state background with ThemeTransparentBrush further ensures uniform theming behavior. Consider verifying that this meets any visual accessibility requirements.
src/Consolonia.Core/Infrastructure/InputLessDefaultNetConsole.cs (1)
87-91: Centralized color handling enhances maintainability
This update neatly hands off color setting to consoleColorMode.SetAttributes, removing the need to manually append ANSI escape codes. Great for modularity and easier maintenance. Ensure all code paths are covered (e.g., italic, underline, strikethrough) so that any decorative text also uses the correct color attributes.
src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs (3)
11-29: Ensure coverage with unit tests for color mapping.
This static array and the logic that references it are critical to your MapToConsoleColor and color-related methods. If you haven't already, consider adding unit tests to validate mapping accuracy, ensuring that each ConsoleColor is mapped to the correct RGB triplet.
Do you want me to generate unit tests that verify these mappings?
31-55: Validate “double shading” logic in Blend.
When both color1 and color2 are EgaColorMode.Shaded, you apply Shade(Shade(...)). This could be intensifying the darkness repeatedly, which might be intentional. Confirm that this behavior matches the intended design and user expectations, especially for multiple layering of shaded colors.
112-124: Reevaluate alpha thresholds.
The thresholds <= 63 for transparency and <= 191 for shading might need fine-tuning if you plan to use semi-transparent colors extensively. Verify that the cutoff values accurately produce the expected visual transitions.
src/Consolonia.Themes/Templates/Controls/DataGrid.axaml (1)
282-283: Confirm new RowBackground fallback behavior.
Switching to {DynamicResource ThemeTransparentBrush} might cause potential styling issues if ThemeTransparentBrush is unavailable or changed. Ensure that the dynamic resource is reliably defined in all themes or consider a default fallback.
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml (3)
5-5: Namespace usage looks good.
The new XML namespace declaration is appropriate for referencing ConsoleColorItem.
20-20: Double-check expanded row definitions.
With "Auto * *", confirm that the newly added row is intended and properly dimensioned, especially if you expect the third row to fill remaining space versus share.
471-495: FourBitControl usage improves color variety demonstration.
The new ItemsControl with a wrap panel and dynamic resource background is a neat way to showcase console colors. Just verify that it maintains consistent layout across various window sizes.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs (8)
14-14: Namespace update to WithLifetimeFixture: Good job keeping tests organized.
This change helps group lifetime-related tests logically. No issues found.
17-17: Removal of IDisposable interface: Confirm cleanup strategy.
Previously, disposing might have handled resource cleanup. Ensure there's no leakage of resources or that torn-down logic is now fully handled elsewhere.
22-22: Null-check on application lifetime: Great improvement for test stability.
This ensures an immediate error if there’s no lifetime, preventing confusing null reference errors down the line.
33-33: Background color changed to Black: Confirm intention.
Originally, many tests checked for Colors.Transparent. Now the test expects Colors.Black. If this aligns with the new color defaults, it’s fine; otherwise, verify if the transition was indeed intended.
331-331: Using Black background for boundary checks in DrawSolidRectangle.
This line enforces background consistency around rectangle edges. Be sure it matches the intended UI style.
337-337: Additional background color check for top/bottom edges:
Same note as above—the black background is consistent, but confirm that’s the desired appearance.
374-374: DrawSingleBox uses black for external regions:
If this aligns with new design guidelines, all good. Otherwise, confirm usage of black for out-of-box area.
470-470: DrawDoubleBox black background for outside-of-box check.
Consistency with single box logic is evident. Just ensure that black is the correct color across all box variants.
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryDataGrid.axaml.cs (1)
602-605: Transitioning from ConsoleBrush to SolidColorBrush: Looks correct.
Using standard Avalonia brushes is more conventional. Consider externalizing these color thresholds if further customization or theming is needed.
src/Consolonia.Core/Drawing/DrawingContextImpl.cs (4)
654-656: Pen property checks simplified: Excellent use of pattern matching.
This ensures only line-style and solid-color brushes pass. The logic is straightforward and readable.
665-670: Extracting nested LineBrush: Clean approach.
Assigning lineStyle directly from the pen’s LineBrush and falling back to the wrapped brush is a neat solution. No concerns here.
672-673: Casting to ISolidColorBrush and returning the color: Straightforward.
This is precisely what we need to finalize color extraction. Just ensure that all pen brush types are truly covered by these checks.
724-724: Foreground brush must be solid color: Raise not supported for others.
This error-handling path ensures we fail fast for unsupported brush types. Good strategy to catch unexpected usage.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/PixelBackgroundTests.cs (4)
6-6: Namespace updated to match WithLifetimeFixture structure.
This keeps test files under a consistent grouping. No issues found.
34-35: Equality checks with Color.Parse: Good replacement for old approach.
Using string-based color parsing simplifies test coverage. This logic is consistent and avoids reliance on enumerations.
49-50: Switch from named color to Color.Parse in inequality tests:
Ensures we’re verifying the new color-based approach. Code clarity remains good.
63-64: Hash code tests rely on parsed colors: Great job.
Confirm that color strings are consistent with previous usage. Otherwise, this is a transparent switch from Mode to direct color usage.
src/.editorconfig (1)
2680-2680: LGTM! Stylistic preference for if conditions.
The change disables ReSharper's suggestion to invert if conditions, which is a valid stylistic choice that doesn't affect functionality.
src/Consolonia.NUnit/UnitTestConsole.cs (1)
74-74: LGTM! Change aligns with color handling simplification
The simplified PixelBackground constructor usage aligns well with the broader initiative to streamline color handling in the codebase. This change in the test console appropriately reflects the removal of PixelBackgroundMode while maintaining the essential color rendering functionality needed for tests.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs
Outdated
Show resolved
Hide resolved
# Conflicts: # src/Consolonia.Core/Drawing/PixelBufferImplementation/Pixel.cs # src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs # src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml.cs # src/Consolonia.Themes/Templates/TurboVisionDefaultColors.axaml
Co-authored-by: <[email protected]>
|
Oh shoot, I moved line brush. Sorry. Arghhh |
…nto 190-strange-coloring-ega-mode
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml (2)
20-21: Consider explicit row height distributionThe current row definitions use "" for both the second and third rows, which might lead to uneven space distribution. Consider using weighted proportions (e.g., "Auto 2 *") if you want to give more space to the color gallery in the second row.
- <Grid RowDefinitions="Auto * *" + <Grid RowDefinitions="Auto 2* *" ColumnDefinitions="* Auto">
482-492: Enhance accessibility for color representationThe current implementation relies solely on visual cues (Rectangle and colored text) to represent colors. Consider adding tooltips or ARIA labels to improve accessibility for users with visual impairments.
<DockPanel LastChildFill="True" Margin="1,1,0,0"> <Rectangle Width="2" Margin="0,0,1,0" Height="1" DockPanel.Dock="Left" - Fill="{Binding Brush}" /> + Fill="{Binding Brush}" + ToolTip="{Binding Name}" /> <TextBlock Text="{Binding Name}" - Foreground="{Binding Brush}" /> + Foreground="{Binding Brush}" + AutomationProperties.Name="{Binding Name}" /> </DockPanel>src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs (3)
8-13: Consider grouping related imports.The imports could be organized into logical groups (Avalonia core, Media, Platform, etc.) to improve readability.
using System; using System.Diagnostics; + using Avalonia; using Avalonia.Media; using Avalonia.Media.TextFormatting; using Avalonia.Platform; + using Consolonia.Controls; using Consolonia.Core.Drawing; using Consolonia.Core.Drawing.PixelBufferImplementation; using Consolonia.Core.Infrastructure; + using NUnit.Framework;
Line range hint
547-567: Consider extracting text drawing setup into a helper method.The
DrawTextmethod contains commented-out code and repeated setup logic that could be extracted into a separate helper method to improve maintainability.+ private static (ITextShaperImpl, IGlyphTypeface) SetupTextDrawing() + { + var textShaper = AvaloniaLocator.Current.GetService<ITextShaperImpl>(); + var fontManager = AvaloniaLocator.Current.GetService<IFontManagerImpl>(); + fontManager.TryCreateGlyphTypeface("Cascadia Mono", FontStyle.Normal, FontWeight.Normal, FontStretch.Normal, + out IGlyphTypeface typeface); + ArgumentNullException.ThrowIfNull(typeface); + return (textShaper, typeface); + } internal static void DrawText(DrawingContextImpl dc, ushort x, ushort y, string text, IBrush brush) { SetOrigin(dc, x, y); - var platformRender = AvaloniaLocator.Current.GetService<IPlatformRenderInterface>(); - var textShaper = AvaloniaLocator.Current.GetService<ITextShaperImpl>(); - var fontManager = AvaloniaLocator.Current.GetService<IFontManagerImpl>(); - fontManager.TryCreateGlyphTypeface("Cascadia Mono", FontStyle.Normal, FontWeight.Normal, FontStretch.Normal, - out IGlyphTypeface typeface); - ArgumentNullException.ThrowIfNull(typeface); + var platformRender = AvaloniaLocator.Current.GetService<IPlatformRenderInterface>(); + var (textShaper, typeface) = SetupTextDrawing(); ShapedBuffer glyphs = textShaper.ShapeText(text.AsMemory(), new TextShaperOptions(typeface, typeface.Metrics.DesignEmHeight)); // Remove commented code IGlyphRunImpl glyphRunImpl = platformRender.CreateGlyphRun(typeface, 1, glyphs, default); dc.DrawGlyphRun(brush, glyphRunImpl); }
4-4: Remove or update the TODO comment.The TODO comment about adding tests is in a test file. Either remove it or specify what additional test coverage is needed.
Would you like me to help create additional test cases or open a GitHub issue to track the missing test coverage?
src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs (4)
31-55: Evaluate additional blending modes if partial alpha blending is needed
Right now, blending uses a simplified model: transparent, shaded, or fully colored. If future requirements involve partial or advanced alpha blending, you may need a more nuanced approach. This is likely acceptable for EGA console constraints, but worth noting.
57-66: Consider expanding support for background transparency or shading
Currently, background colors must beColored, otherwise a not-supported exception is raised. If there's a use case for transparent or shaded backgrounds, you could implement fallback logic instead of throwing an exception.
65-66: Unusedweightparameter
Theweightparameter is accepted but not utilized. If future features require handling bold or other font weights, consider implementing or removing this parameter to maintain clarity.
112-124: Alpha threshold assumptions
The boundaries (<=63 → Transparent, <=191 → Shaded, else Colored) are simplistic but can be effective for a console environment. Ensure that these threshold values match user expectations for color transitions.src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs (1)
30-31: ChainingUseAutoDetectConsoleColorMode
Auto-detecting the color mode immediately after selecting the console instance offers a clean entry point. Confirm that you do not need to conditionally disable auto-detection under special circumstances (e.g. restricted console environments).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/Consolonia.Core/ApplicationStartup.cs(4 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs(0 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/RgbConsoleColorMode.cs(1 hunks)src/Consolonia.Core/Drawing/RenderTarget.cs(1 hunks)src/Consolonia.Core/Infrastructure/InputLessDefaultNetConsole.cs(2 hunks)src/Consolonia.Gallery/App.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml(3 hunks)src/Consolonia.Gallery/View/ControlsListView.axaml(1 hunks)src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs(2 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/Assembly.cs(1 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs(5 hunks)src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs(1 hunks)
💤 Files with no reviewable changes (1)
- src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBackgroundMode.cs
🚧 Files skipped from review as they are similar to previous changes (9)
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/Assembly.cs
- src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaColorMode.cs
- src/Consolonia.Gallery/View/ControlsListView.axaml
- src/Consolonia.Core/Drawing/RenderTarget.cs
- src/Consolonia.Gallery/App.cs
- src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs
- src/Consolonia.Core/Drawing/PixelBufferImplementation/RgbConsoleColorMode.cs
- src/Consolonia.Core/ApplicationStartup.cs
- src/Consolonia.Core/Infrastructure/InputLessDefaultNetConsole.cs
🔇 Additional comments (12)
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml (3)
5-5: LGTM: Namespace declaration is properly defined
The namespace declaration follows XAML conventions and is correctly scoped to the project's namespace.
471-495: LGTM: Well-structured ItemsControl implementation
The ItemsControl implementation is well-structured and properly integrates with the EGA color mode:
- Uses appropriate data templating
- Implements vertical wrapping for compact display
- Properly binds to ConsoleColorItem properties
- Uses DynamicResource for theme-aware background
472-474: Review fixed height constraint
The fixed height of 16 units might cause content overflow if there are many color items, especially with vertical orientation. Consider making the height responsive to content or adding a ScrollViewer.
src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs (3)
332-338: Color assertions consistently updated across tests.
The background color assertions have been updated from Colors.Transparent to Colors.Black across multiple test methods, maintaining consistency with the new color mode system.
Let's verify if any color assertions were missed:
Also applies to: 375-375, 471-471
✅ Verification successful
Let me search for all color assertions to verify the consistency of the changes and ensure no transparent background colors were missed.
Background color assertions are consistently updated
After analyzing all color assertions in the test files, I can confirm that all background color assertions have been properly updated. While there are still Colors.Transparent assertions present, they are all used for foreground colors, which is correct. All background color assertions now consistently use either Colors.Black or Colors.Blue as expected with the new color mode system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining Transparent color assertions
rg "Assert.*Background.*Color.*==.*Colors\.Transparent" --type cs
Length of output: 67
Script:
#!/bin/bash
# Search for all color assertions in test files to get a comprehensive view
rg "Assert.*Color.*==.*Colors\." --type cs
Length of output: 9959
15-19: Class signature simplified by removing IDisposable.
The removal of the IDisposable interface suggests that resource cleanup is now handled differently, possibly at a higher level. This aligns with the changes mentioned in the AI summary.
Let's verify there are no remaining resource cleanup requirements:
✅ Verification successful
Let's gather more specific information about the resource management changes.
Let's check one more aspect to understand how the test cleanup is now managed.
Removal of IDisposable is safe as cleanup is handled by LifetimeSetupFixture
Based on the verification results, we can see that:
- The
DrawingContextImplclass itself doesn't implement IDisposable or contain cleanup patterns - Resource cleanup is now managed by
LifetimeSetupFixturewhich implements IDisposable and handles the lifetime scope - The test class inherits from this fixture setup, ensuring proper resource management at the fixture level
This architectural change improves the separation of concerns by moving resource management to a dedicated fixture class.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any cleanup patterns or IDisposable implementations in related test files
rg -l "IDisposable|Dispose\(\)|using.*new.*ConsoleWindow" --type cs
Length of output: 1454
Script:
#!/bin/bash
# Check the implementation of DrawingContextImpl for any cleanup patterns
rg -A 5 "class DrawingContextImpl" src/Consolonia.Core/Drawing/DrawingContextImpl.cs
# Check if there are any using statements or cleanup in the test file
rg "using.*new DrawingContextImpl|Dispose\(\)" src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs
# Check if LifetimeSetupFixture handles cleanup
rg -A 10 "class LifetimeSetupFixture" src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/LifetimeSetupFixture.cs
Length of output: 1083
Script:
#!/bin/bash
# Check test fixture setup and teardown
rg -B 2 -A 10 "\[SetUp\]|\[TearDown\]|\[OneTime" src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs
# Check if there are any using statements with DrawingContextImpl
rg "DrawingContextImpl.*=.*new" src/Tests/Consolonia.Core.Tests/WithLifetimeFixture/DrawingContextImplTests.cs
Length of output: 242
Line range hint 23-35: Buffer initialization test updated for new color mode.
The test now expects Colors.Black as the default background color instead of Colors.Transparent. This change appears to be related to the new color mode system mentioned in the AI summary.
Let's verify the consistency of this change across the codebase:
✅ Verification successful
Let me search for the actual implementation of the buffer initialization to verify this behavior.
Let me try a different approach to find the implementation.
Let me check the actual implementation of the PixelBuffer initialization.
Let me check the actual implementation of the Pixel constructor and default values.
Based on the search results, I can now provide a definitive response about the buffer initialization behavior:
The change to Colors.Black as default background color is consistent with the codebase.
Looking at the implementation in src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBuffer.cs, we can see that the buffer is explicitly initialized with black background:
_buffer[x, y] = new Pixel(new PixelBackground(Colors.Black));This is the default initialization for all pixels in the buffer, and all tests across the codebase consistently verify this behavior. The test's expectation of Colors.Black as the default background color is correct and matches the actual implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other color-related assertions
rg "Assert.*Color.*=.*Colors\.(Transparent|Black)" --type cs
Length of output: 2909
Script:
#!/bin/bash
# Search for ConsoleWindow and PixelBuffer initialization
ast-grep --pattern 'class ConsoleWindow {
$$$
}'
# Also search for PixelBuffer class definition
ast-grep --pattern 'class PixelBuffer {
$$$
}'
Length of output: 103
Script:
#!/bin/bash
# Search for ConsoleWindow and PixelBuffer files
fd -e cs "ConsoleWindow|PixelBuffer"
# Search for buffer initialization code
rg -A 5 "new (ConsoleWindow|PixelBuffer)" --type cs
Length of output: 23190
Script:
#!/bin/bash
# Check the PixelBuffer constructor implementation
rg -A 10 "class PixelBuffer" --type cs
Length of output: 3772
Script:
#!/bin/bash
# Check the Pixel implementation
rg -A 15 "class Pixel" --type cs
# Also check for default pixel creation
rg "new Pixel\(" -A 3 --type cs
Length of output: 32014
src/Consolonia.Core/Drawing/PixelBufferImplementation/EgaConsoleColor/EgaConsoleColorMode.cs (5)
1-6: All imported namespaces appear necessary and relevant
The use of System, System.Linq, System.Text, and Avalonia.Media is clearly reflected in the code (.OrderBy, StringBuilder, Color type), making these imports appropriate.
11-29: Validate the accuracy of the color-to-RGB mappings
The static ConsoleColorMap array needs to precisely match any expected EGA or standard console color definitions. If you rely on consistent color representation across different platforms, consider verifying each RGB triple's correctness to avoid unintended color shifts or mismatches.
70-110: ANSI escape codes
The approach for generating ANSI escape codes looks correct. Since Windows 10+ supports VT sequences by default, this is likely fine. For older systems, confirm that virtual terminal processing is enabled (if needed).
136-158: Shading logic
This shading approach forces multiple console colors to eventually become Black. Confirm that repeatedly shading colors like DarkGray or DarkYellow is correct for your use case.
160-175: Mapped alpha values
The method correctly sets alpha for transparent (0) or shaded (127). This is a fixed approach for “semi-transparency.” If more nuanced transparency levels are required, consider bridging them more granularly.
src/Consolonia.PlatformSupport/PlatformSupportExtensions.cs (1)
4-5: New references for color mode logic
These references to PixelBufferImplementation and EgaConsoleColor indicate that color-specific logic has shifted into separate classes. This separation of concerns is a good design decision.
| ConsoleColor.Magenta => 13, | ||
| ConsoleColor.Cyan => 14, | ||
| ConsoleColor.White => 15, | ||
| _ => 7 // Default to white if unknown |
There was a problem hiding this comment.
is it possible to detect default color when app starts? Maybe future enhancement.
| $"\x1b[{(isBackground ? 40 + ansiCode : 30 + ansiCode)}m" | ||
| : | ||
| // Bright colors | ||
| $"\x1b[{(isBackground ? 100 + (ansiCode - 8) : 90 + (ansiCode - 8))}m"; |
|
|
||
| return Color.FromArgb(a, r, g, b); | ||
| var consoleColorMode = AvaloniaLocator.Current.GetRequiredService<IConsoleColorMode>(); | ||
| return consoleColorMode.Blend(target, source); |
| @@ -26,7 +27,7 @@ public PixelBuffer(ushort width, ushort height) | |||
| // blended into it. | |||
| for (ushort y = 0; y < height; y++) | |||
| for (ushort x = 0; x < width; x++) | |||
| _buffer[x, y] = Pixel.Space; | |||
| _buffer[x, y] = new Pixel(new PixelBackground(Colors.Black)); | |||
There was a problem hiding this comment.
Why not transparent background?
No description provided.