Move LineBrush/LineStyle into Consolonia.Controls public namespace #232
Conversation
📝 WalkthroughWalkthroughThis pull request involves a comprehensive namespace reorganization within the Consolonia project, primarily focusing on moving drawing and control-related classes from Possibly related issues
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (7)
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: 0
🧹 Nitpick comments (4)
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml.cs (1)
17-17: Consider usingrnd.Next(256)to include the full range of byte values.Currently,
rnd.Next(255)generates numbers in the range[0..254]. If the intention is to allow a channel value of up to 255, update it tornd.Next(256).src/Consolonia.Core/Controls/ConsoloniaAccessText.cs (2)
31-49: Consider optimizing string operations and handling edge cases.The current implementation could benefit from the following improvements:
- Performance optimization for string operations
- Handling of multiple underscores
- Support for escaped underscores (e.g., "__" to display "_")
Consider this alternative implementation:
if (!string.IsNullOrEmpty(Text)) { - var inlines = new InlineCollection(); - int iPos = Text.IndexOf('_', StringComparison.Ordinal); - if (iPos >= 0 && iPos < Text.Length - 1) - { - inlines.Add(new Run(Text[..iPos])); - _accessRun = new Run(Text[++iPos..++iPos]); - inlines.Add(_accessRun); - inlines.Add(new Run(Text[iPos..])); - } - else - { - _accessRun = null; - inlines.Add(new Run(Text)); - } - - Inlines = inlines; + var inlines = new InlineCollection(); + var currentPos = 0; + var textLength = Text.Length; + + while (currentPos < textLength) + { + var underscorePos = Text.IndexOf('_', currentPos, StringComparison.Ordinal); + if (underscorePos == -1 || underscorePos == textLength - 1) + { + // No more underscores or underscore at end - add remaining text + inlines.Add(new Run(Text[currentPos..])); + _accessRun = null; + break; + } + + // Handle text before underscore + if (underscorePos > currentPos) + inlines.Add(new Run(Text[currentPos..underscorePos])); + + // Check for escaped underscore + if (underscorePos < textLength - 1 && Text[underscorePos + 1] == '_') + { + inlines.Add(new Run("_")); + currentPos = underscorePos + 2; + continue; + } + + // Handle access key + _accessRun = new Run(Text[(underscorePos + 1)..(underscorePos + 2)]); + inlines.Add(_accessRun); + currentPos = underscorePos + 2; + } + + Inlines = inlines; }This implementation:
- Handles multiple underscores correctly
- Supports escaped underscores
- Processes the string in a single pass
- Improves code readability with meaningful variable names
37-40: Improve string slicing readability.The current string slicing operations are concise but could be more readable.
Consider this alternative:
-inlines.Add(new Run(Text[..iPos])); -_accessRun = new Run(Text[++iPos..++iPos]); -inlines.Add(_accessRun); -inlines.Add(new Run(Text[iPos..])); +// Add text before the underscore +inlines.Add(new Run(Text.Substring(0, iPos))); + +// Move past underscore and get access key character +iPos++; +_accessRun = new Run(Text.Substring(iPos, 1)); +inlines.Add(_accessRun); + +// Add remaining text +iPos++; +inlines.Add(new Run(Text.Substring(iPos)));src/Consolonia.Gallery/Gallery/GalleryViews/GalleryScrollViewer.axaml.cs (1)
69-69: Consider streamlining the split logic for multiline strings.Using
(char[])['\r', '\n']may be less intuitive than creating a newchar[] { '\r', '\n' }. While both are valid, the more conventional approach could improve readability.Example fix:
- text.AddRange(""" ... """.Split((char[])['\r', '\n'], StringSplitOptions.RemoveEmptyEntries)); + text.AddRange(""" ... """.Split(new[] { '\r', '\n' }, StringSplitOptions.RemoveEmptyEntries));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
src/Consolonia.Core/Controls/ConsoloniaAccessText.cs(1 hunks)src/Consolonia.Core/Controls/Dialog/DialogHelpers.cs(1 hunks)src/Consolonia.Core/Controls/Dialog/DialogWrap.axaml.cs(1 hunks)src/Consolonia.Core/Controls/DialogWindow.cs(1 hunks)src/Consolonia.Core/Controls/FileOpenPicker.axaml(1 hunks)src/Consolonia.Core/Controls/FileOpenPicker.axaml.cs(1 hunks)src/Consolonia.Core/Controls/FileOpenPickerViewModel.cs(1 hunks)src/Consolonia.Core/Controls/FileSavePicker.axaml(1 hunks)src/Consolonia.Core/Controls/FileSavePicker.axaml.cs(1 hunks)src/Consolonia.Core/Controls/FileSavePickerViewModel.cs(1 hunks)src/Consolonia.Core/Controls/FolderPicker.axaml(1 hunks)src/Consolonia.Core/Controls/FolderPicker.axaml.cs(1 hunks)src/Consolonia.Core/Controls/LineBrush.cs(1 hunks)src/Consolonia.Core/Controls/LineStyle.cs(1 hunks)src/Consolonia.Core/Controls/MessageBox.axaml(1 hunks)src/Consolonia.Core/Controls/MessageBox.axaml.cs(1 hunks)src/Consolonia.Core/Drawing/BrushExtensions.cs(1 hunks)src/Consolonia.Core/Drawing/DrawingContextImpl.cs(7 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/DrawingBoxSymbol.cs(2 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBuffer.cs(2 hunks)src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBufferConverter.cs(2 hunks)src/Consolonia.Core/Drawing/RenderTarget.cs(2 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryAnimatedLines.axaml.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryColors.axaml.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryDialog.axaml.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryMessageBox.axaml.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/GalleryScrollViewer.axaml.cs(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/SomeDialogWindow.axaml(1 hunks)src/Consolonia.Gallery/Gallery/GalleryViews/SomeDialogWindow.axaml.cs(1 hunks)src/Consolonia.Gallery/View/XamlDialogWindow.axaml(1 hunks)src/Consolonia.Gallery/View/XamlDialogWindow.axaml.cs(1 hunks)src/Consolonia.Themes/Templates/Controls/DialogWindow.axaml(2 hunks)src/Consolonia.Themes/TurboVisionDark/Controls/DialogWindow.axaml(1 hunks)src/Example.Blazor.Components/MainPage.razor(1 hunks)
✅ Files skipped from review due to trivial changes (17)
- src/Consolonia.Core/Controls/FolderPicker.axaml.cs
- src/Consolonia.Gallery/Gallery/GalleryViews/GalleryDialog.axaml.cs
- src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBuffer.cs
- src/Consolonia.Core/Controls/FileOpenPickerViewModel.cs
- src/Consolonia.Gallery/Gallery/GalleryViews/GalleryMessageBox.axaml.cs
- src/Consolonia.Gallery/View/XamlDialogWindow.axaml.cs
- src/Consolonia.Core/Drawing/PixelBufferImplementation/PixelBufferConverter.cs
- src/Consolonia.Core/Drawing/RenderTarget.cs
- src/Consolonia.Core/Controls/Dialog/DialogHelpers.cs
- src/Consolonia.Core/Controls/FileSavePicker.axaml.cs
- src/Consolonia.Core/Controls/MessageBox.axaml.cs
- src/Consolonia.Core/Controls/Dialog/DialogWrap.axaml.cs
- src/Consolonia.Core/Controls/FileOpenPicker.axaml.cs
- src/Consolonia.Core/Controls/DialogWindow.cs
- src/Consolonia.Gallery/Gallery/GalleryViews/GalleryAnimatedLines.axaml.cs
- src/Example.Blazor.Components/MainPage.razor
- src/Consolonia.Core/Drawing/BrushExtensions.cs
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Consolonia.Core/Drawing/DrawingContextImpl.cs
- src/Consolonia.Core/Controls/LineBrush.cs
- src/Consolonia.Core/Controls/LineStyle.cs
- src/Consolonia.Core/Drawing/PixelBufferImplementation/DrawingBoxSymbol.cs
- src/Consolonia.Themes/Templates/Controls/DialogWindow.axaml
🔇 Additional comments (13)
src/Consolonia.Gallery/Gallery/GalleryViews/SomeDialogWindow.axaml (1)
6-6: Namespace refactor is aligned with the PR objectives.
Changing the namespace to Consolonia.Controls is consistent with the overall refactoring effort in this pull request. Ensure all references to the old namespace are removed or updated accordingly throughout the project.
src/Consolonia.Gallery/Gallery/GalleryViews/SomeDialogWindow.axaml.cs (1)
4-4: Namespace import change is consistent.
Switching from Consolonia.Core.Controls to Consolonia.Controls matches the new organizational structure. Looks good.
src/Consolonia.Gallery/View/XamlDialogWindow.axaml (1)
6-6: Updated namespace to Consolonia.Controls looks consistent with the refactoring goals.
This is aligned with the broader reorganization from Consolonia.Core.Controls to Consolonia.Controls. Please confirm that other references in XAML or code also point to the new namespace to avoid potential bindings or type resolution issues.
src/Consolonia.Themes/TurboVisionDark/Controls/DialogWindow.axaml (1)
3-3: LGTM! The updated namespace aligns with the PR objectives.
The usage of controls="clr-namespace:Consolonia.Controls;assembly=Consolonia.Core" is consistent with the newly consolidated controls namespace. Ensure references to DialogWindow (and related controls) in other code files and XAML resources are also updated to maintain consistency.
src/Consolonia.Core/Controls/FolderPicker.axaml (2)
4-5: Namespace reorganization looks good.
By introducing Consolonia.Controls and Consolonia.Core.Controls, you’ve clearly separated core components from higher-level ones. This helps maintain a cleaner architecture and organizes code more effectively.
8-8: Ensure all bindings remain valid.
Please confirm that updating x:DataType references (e.g., controls:FolderPickerViewModel → core-controls:FolderPickerViewModel) does not break any view model data binding or constructor injection.
src/Consolonia.Core/Controls/FileSavePicker.axaml (2)
4-5: Namespace move is consistent.
Switching to Consolonia.Controls and introducing core-controls to reference Consolonia.Core.Controls clarifies library boundaries and fosters maintainability.
8-8: Confirm updated view model references.
Make sure the new data type (core-controls:FileSavePickerViewModel) and its bindings are properly recognized by related C# code, including any code-behind or tests referencing the former namespace.
src/Consolonia.Core/Controls/FileOpenPicker.axaml (2)
4-5: Consistent namespace update.
Keeping Consolonia.Controls for UI elements and Consolonia.Core.Controls for core functionalities aligns with the project’s apparent layering scheme. Great step for code clarity.
8-8: Validate the changed data type binding.
Verify that the FileOpenPickerViewModel references are correctly updated throughout the solution to the new core-controls namespace.
src/Consolonia.Core/Controls/MessageBox.axaml (2)
3-3: Consistent namespace update.
This change aligns with the refactoring objective of moving controls into the Consolonia.Controls namespace. It looks good.
7-7: Confirm references to the renamed class.
Changing the x:Class attribute to Consolonia.Controls.MessageBox may require updates wherever this class is instantiated or referenced.
Use this script to confirm no stale references remain:
✅ Verification successful
No stale references found, namespace change is safe
The search results show that all references to MessageBox are using the new namespace Consolonia.Controls.MessageBox. The class is being used correctly in:
src/Example.Blazor.Components/MainPage.razorsrc/Consolonia.Blazor/readme.mdsrc/Consolonia.Gallery/Gallery/GalleryViews/GalleryMessageBox.axaml.cs
There are no references to the old namespace Consolonia.Core.Controls.MessageBox, indicating the namespace change is safe and all references have been updated correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find old references to the former `Consolonia.Core.Controls.MessageBox`.
rg -A 3 "Consolonia\.Core\.Controls\.MessageBox"
Length of output: 50
Script:
#!/bin/bash
# Let's search for any references to MessageBox class in the codebase
rg -A 3 "MessageBox"
# Also search for any XAML references
fd -e xaml -x rg -l "MessageBox"
# Let's also check for any using statements or imports
rg "using.*MessageBox"
Length of output: 14416
src/Consolonia.Gallery/Gallery/GalleryViews/GalleryScrollViewer.axaml.cs (1)
70-70: Ensure the spread operator aligns with expected .NET version.
Using the C# range spread operator ([.. text]) requires a recent language version. Confirm that the target framework supports this syntax to avoid potential build or runtime issues.
To verify usage, run the following search script to see if there are any references to language versions:
✅ Verification successful
Collection initializer syntax is supported in the codebase's target framework
Based on the project files analysis, the codebase is using modern .NET features:
- The Directory.Core.Build.props shows analysis level 6.0
- Multiple projects reference modern packages and use current SDK features
- The collection initializer syntax
[.. text]is supported since C# 8.0, which is well within the capabilities of the current setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Checking for usage of <LangVersion> in .csproj files or Directory.Build.props
rg -A 3 -B 3 "<LangVersion>"
Length of output: 30
Script:
#!/bin/bash
# Let's check the project files for target framework version and other relevant settings
fd -e csproj . -x cat {}
Length of output: 13669
Script:
#!/bin/bash
# Let's check the Directory.Core.Build.props file since many projects import it
fd -g "Directory.Core.Build.props" -x cat {}
Length of output: 988
Script:
#!/bin/bash
# Let's check the global.json file for SDK version
cat global.json 2>/dev/null || echo "No global.json found"
# Also check the target framework in the specific project
fd -g "Consolonia.Gallery.csproj" -x cat {}
Length of output: 863
also moved DialogWindow, MessageBox