-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Refactor for cross platform port (#3641) #3642
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
base: master
Are you sure you want to change the base?
Conversation
|
Why that moving around? namespace ICSharpCode.ILSpy.TextView => namespace ICSharpCode.ILSpy.TextViewControl |
|
Project Rover uses two compiler tricks to enable source file reuse without heavy modifications, However, such tricks fail easily when a type has the same name as an existing namespace. The term I will see how much effort it takes to avoid this namespace change though. Will provide an update once I have some results. |
|
Reverted the namespace changes, but one more extra file to maintain in the cross platform port. |
1b833df to
a0f38ad
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request refactors the ILSpy codebase to support cross-platform porting by separating WPF-specific code from platform-independent logic. The refactoring maintains existing functionality while restructuring code to enable easier platform abstraction.
Key Changes
- Split multiple classes into platform-specific (.wpf.cs) and shared (.cs) files to isolate WPF dependencies
- Extracted nested classes and extension methods into separate files for better modularity and platform-specific replacement
- Added IList interface implementation to SharpTreeNodeCollection for cross-platform binding compatibility
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| ILSpy/Views/OpenFromGacDialog.xaml.cs | Removed nested GacEntry class (extracted to separate file) and unused using statement |
| ILSpy/Views/GacEntry.cs | New file containing extracted GacEntry class nested within OpenFromGacDialog partial class |
| ILSpy/ViewModels/TabPageModelExtensions.wpf.cs | New file with WPF-specific Focus extension method for TabPageModel |
| ILSpy/ViewModels/TabPageModelExtensions.cs | New file with platform-independent TabPageModel extension methods |
| ILSpy/ViewModels/TabPageModel.cs | Removed TabPageModelExtensions static class (extracted to separate files) and unused using statements |
| ILSpy/ViewModels/PaneModel.cs | Removed Pane static class with WPF dependency properties (extracted to separate file) and unused using statement |
| ILSpy/ViewModels/Pane.cs | New file containing extracted Pane static class with WPF dependency properties |
| ILSpy/ViewModels/LegacyToolPaneModel.cs | Removed unused System.Windows using statement |
| ILSpy/ViewModels/DebugStepsPaneModel.cs | Removed unused System.Windows using statement |
| ILSpy/TreeNodes/AssemblyListTreeNode.wpf.cs | New file with WPF-specific drag-drop implementation for AssemblyListTreeNode |
| ILSpy/TreeNodes/AssemblyListTreeNode.cs | Made class partial, removed drag-drop methods (moved to .wpf.cs file) and unused using statements |
| ILSpy/TextView/VisualLineReferenceText.wpf.cs | New file with WPF-specific cursor handling for VisualLineReferenceText |
| ILSpy/TextView/VisualLineReferenceText.cs | New file with platform-independent VisualLineReferenceText implementation |
| ILSpy/TextView/ViewState.cs | New file containing extracted ViewState class from DecompilerTextView |
| ILSpy/TextView/ReferenceElementGenerator.cs | Removed nested VisualLineReferenceText class (extracted to separate files) |
| ILSpy/TextView/IBracketSearcher.cs | New file containing extracted IBracketSearcher interface |
| ILSpy/TextView/DefaultBracketSearcher.cs | New file containing extracted DefaultBracketSearcher class |
| ILSpy/TextView/DecompilerTextView.cs | Removed ViewState class (extracted to separate file) |
| ILSpy/TextView/BracketSearchResult.cs | New file containing extracted BracketSearchResult class |
| ILSpy/TextView/BracketHighlightRenderer.cs | Removed bracket-related classes (extracted to separate files) and added null-safety for resource loading |
| ILSpy/SmartTextOutputExtensions.cs | New file containing extracted SmartTextOutputExtensions static class |
| ILSpy/Search/ShowSearchCommand.cs | New file containing extracted ShowSearchCommand class with debug logging |
| ILSpy/Search/SearchPaneModel.cs | Added [Export] attribute and made class partial |
| ILSpy/Search/SearchPane.xaml.cs | Removed ShowSearchCommand class (extracted to separate file) and unused using statement |
| ILSpy/Options/DisplaySettings.cs | Added CROSS_PLATFORM conditional compilation for font handling |
| ILSpy/Languages/CSharpLanguage.wpf.cs | New file with WPF-specific AddWarningMessage implementation |
| ILSpy/Languages/CSharpLanguage.cs | Made class partial and removed AddWarningMessage method (moved to .wpf.cs file) |
| ILSpy/ISmartTextOutput.cs | Removed SmartTextOutputExtensions class (extracted to separate file) and unused using statements |
| ILSpy/Docking/DockWorkspace.wpf.cs | New file with WPF-specific docking and layout management implementation |
| ILSpy/Docking/DockWorkspace.cs | Made class partial, changed Export attribute to explicit type, removed WPF-specific methods and using statements |
| ILSpy/AssemblyTree/AssemblyTreeModel.wpf.cs | New file with WPF-specific constructor and initialization logic |
| ILSpy/AssemblyTree/AssemblyTreeModel.cs | Made class partial, removed constructor and LoadInitialAssemblies method, added TODO comments |
| ILSpy/AboutPage.cs | Added CROSS_PLATFORM conditional compilation for binding and image loading |
| ICSharpCode.ILSpyX/TreeView/SharpTreeNodeCollection.cs | Added IList interface implementation with collection manipulation methods |
| ICSharpCode.ILSpyX/TreeView/SharpTreeNode.cs | Added ViewChildren property and LoadingTreeNode class for cross-platform tree view binding |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Fyi, we run Copilot reviews on almost all PRs these days to see how it changes over time (quality-wise), so expect nuggets and duds. |
|
@christophwille It actually provides some useful insights. I am still working on the repo reconfiguration, so will take care of them later. |
|
No need to rush anything. A well-thought-out & tested approach is appreciated. |
d571676 to
175fd60
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 55 out of 55 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Typical changes needed by the cross platform port are,
Still a work-in-progress, but should be ready to review and merge as it doesn't change anything fundamental on the WPF side.