Dependency Injection Code Quality #3276
Conversation
This comment has been minimized.
This comment has been minimized.
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThis pull request centralizes the translation retrieval mechanism throughout the codebase. Calls to the deprecated and obsolete Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "UI Component"
participant API as "App.API"
participant DI as "Ioc/DI Container"
participant TS as "Translation Service"
UI->>API: GetTranslation(key)
API->>DI: Resolve Internationalization instance
DI-->>API: Return Internationalization instance
API->>TS: Fetch translation for key
TS-->>API: Return translation string
API-->>UI: Provide translated string
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)
✨ Finishing Touches
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 (1)
Flow.Launcher/PriorityChangeWindow.xaml.cs (1)
56-56: Good use of pattern matching.Changed from a traditional type check and cast to pattern matching, which is a more modern and concise C# syntax. This improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
Flow.Launcher.Core/ExternalPlugins/Environments/AbstractPluginEnvironment.cs(2 hunks)Flow.Launcher.Core/Plugin/PluginManager.cs(8 hunks)Flow.Launcher.Core/Resource/InternationalizationManager.cs(0 hunks)Flow.Launcher.Core/Resource/LocalizedDescriptionAttribute.cs(1 hunks)Flow.Launcher.Core/Resource/Theme.cs(2 hunks)Flow.Launcher.Core/Resource/ThemeManager.cs(0 hunks)Flow.Launcher.Core/Resource/TranslationConverter.cs(1 hunks)Flow.Launcher.Core/Updater.cs(1 hunks)Flow.Launcher/ActionKeywords.xaml.cs(1 hunks)Flow.Launcher/App.xaml.cs(3 hunks)Flow.Launcher/Converters/TextConverter.cs(1 hunks)Flow.Launcher/CustomQueryHotkeySetting.xaml.cs(2 hunks)Flow.Launcher/CustomShortcutSetting.xaml.cs(2 hunks)Flow.Launcher/Helper/HotKeyMapper.cs(3 hunks)Flow.Launcher/HotkeyControl.xaml.cs(1 hunks)Flow.Launcher/HotkeyControlDialog.xaml.cs(4 hunks)Flow.Launcher/MainWindow.xaml.cs(3 hunks)Flow.Launcher/PriorityChangeWindow.xaml.cs(2 hunks)Flow.Launcher/PublicAPIInstance.cs(2 hunks)Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs(3 hunks)Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs(2 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs(5 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs(6 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneHotkeyViewModel.cs(4 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneProxyViewModel.cs(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs(12 hunks)Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml.cs(1 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(6 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs(2 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs(3 hunks)
💤 Files with no reviewable changes (2)
- Flow.Launcher.Core/Resource/ThemeManager.cs
- Flow.Launcher.Core/Resource/InternationalizationManager.cs
✅ Files skipped from review due to trivial changes (1)
- Flow.Launcher/Helper/HotKeyMapper.cs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (73)
Flow.Launcher/SettingPages/Views/SettingsPanePluginStore.xaml.cs (1)
51-51: Implementation aligns with dependency injection improvement goals.The change from
PluginManager.API.OpenUrltoApp.API.OpenUrlcorrectly implements the PR objective of making PluginManager.API private and consolidating API access through the centralized App.API interface.Flow.Launcher/SettingPages/ViewModels/SettingsPaneProxyViewModel.cs (1)
24-24: Successfully refactored translation access.This change correctly implements the PR objective of removing direct dependency on InternationalizationManager.Instance by using the App.API.GetTranslation method instead.
Flow.Launcher/ViewModel/PluginViewModel.cs (4)
105-106: Translation access properly refactored.Successfully replaced InternationalizationManager.Instance.GetTranslation with App.API.GetTranslation for string localization, aligning with the dependency injection improvements.
136-136: API access consolidated through App.API.The change from PluginManager.API.OpenDirectory to App.API.OpenDirectory properly implements the goal of making PluginManager.API private.
142-142: API access consolidated through App.API.The change from PluginManager.API.OpenUrl to App.API.OpenUrl correctly implements the encapsulation objective.
148-149: API access properly consolidated.Successfully refactored to use App.API for ChangeQuery and ShowMainWindow operations, improving encapsulation and dependency management.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneHotkeyViewModel.cs (4)
43-43: Translation access properly refactored in CustomHotkeyDelete.Successfully replaced InternationalizationManager.Instance.GetTranslation with App.API.GetTranslation for multiple translation strings, improving dependency management.
Also applies to: 49-50, 51-51
68-68: Translation access properly refactored in CustomHotkeyEdit.Correctly implemented the dependency injection approach for translations by using App.API.GetTranslation.
89-89: Translation access properly refactored in CustomShortcutDelete.Successfully replaced InternationalizationManager.Instance.GetTranslation with App.API.GetTranslation for multiple translation strings, improving dependency management.
Also applies to: 95-96, 97-97
113-113: Translation access properly refactored in CustomShortcutEdit.Correctly implemented the dependency injection approach for translations by using App.API.GetTranslation.
Flow.Launcher/CustomQueryHotkeySetting.xaml.cs (2)
62-62: Notification interface change looks correct.Changed from direct usage of
InternationalizationManager.InstancetoApp.API.GetTranslationfor retrieving translations, which aligns with the PR objectives to remove InternationalizationManager.Instance in favor of API functions.
70-70: Translation change is consistent.Using
App.API.GetTranslationfor consistent access to translations throughout the codebase.Flow.Launcher/Converters/TextConverter.cs (1)
25-25: Good translation layer migration.Changed from
InternationalizationManager.Instance.GetTranslationtoApp.API.GetTranslationfor consistency across the codebase, aligning with the PR objectives for dependency injection.Flow.Launcher.Core/Resource/Theme.cs (2)
117-117: Correctly using injected API for translations.Changed from using the global
InternationalizationManager.Instanceto using the injected_apiinterface for translations, which is a good practice for dependency injection and makes the code more testable.
127-127: Consistent use of injected API for translations.Similar to the previous change, using the injected
_apiinterface for translations instead of the singleton pattern.Flow.Launcher/PriorityChangeWindow.xaml.cs (2)
24-24: Translation access correctly updated.Changed from using
InternationalizationManager.InstancetoApp.API.GetTranslationfor retrieving translations, which aligns with the PR objectives.
43-44: Improved translation handling.Simplified by directly using
App.API.GetTranslationinstead of storing the translation in a local variable first. This reduces unnecessary variables and makes the code more direct.Flow.Launcher.Core/Updater.cs (1)
148-148: LGTM: Translation retrieval updated to use dependency-injected APIThis change correctly transitions from using the deprecated InternationalizationManager singleton to the injected
_apiinstance for retrieving translations, which aligns with the PR's dependency injection improvements.Flow.Launcher.Core/Resource/LocalizedDescriptionAttribute.cs (3)
2-3: Added necessary import statementsAdded imports for DI container and public API interface.
9-9: Updated translation source to use dependency injectionProperly retrieving the IPublicAPI from the DI container instead of using the deprecated InternationalizationManager singleton.
21-21: Updated translation retrieval methodTranslation lookup is now correctly performed using the injected API service instead of the previous translation manager.
Flow.Launcher.Core/ExternalPlugins/Environments/AbstractPluginEnvironment.cs (1)
57-58: Translation retrieval updated to use injected APISuccessfully replaced InternationalizationManager.Instance with the injected API instance for retrieving translations throughout the setup method.
Also applies to: 64-65, 87-87
Flow.Launcher.Core/Resource/TranslationConverter.cs (4)
4-5: Added necessary imports for dependency injection.The added imports make it clear that this file now uses dependency injection through the CommunityToolkit.Mvvm package to obtain the IPublicAPI interface.
11-11: Properly injected IPublicAPI instance.This change aligns with the PR objective to remove direct InternationalizationManager.Instance usage in favor of a more maintainable dependency injection approach.
16-18: Updated string handling and translation method.Good changes here:
- Updated to lowercase
string(preferred C# style)- Now using the injected API for translation instead of the previous singleton
This supports the dependency injection improvements targeted by this PR.
21-21: Simplified exception handling.The simplified ConvertBack implementation is cleaner while maintaining the same functionality.
Flow.Launcher/HotkeyControl.xaml.cs (1)
116-116: Removed dependency on InternationalizationManager singleton.Updated to use App.API.GetTranslation instead of direct singleton access, which matches the PR objective of removing InternationalizationManager.Instance usage.
Flow.Launcher/Resources/Pages/WelcomePage1.xaml.cs (3)
6-6: Added necessary import for dependency injection.Added the CommunityToolkit.Mvvm.DependencyInjection namespace to support the IoC container usage in this class.
21-22: Implemented proper dependency injection for Internationalization service.Added a private readonly field that obtains the Internationalization service through dependency injection, which aligns with the PR objective to remove direct InternationalizationManager.Instance usage.
34-37: Updated translation service access.Now using the injected _translater instance instead of the previous singleton approach for both ChangeLanguage and PromptShouldUsePinyin methods, which is consistent with the PR's dependency injection goals.
Flow.Launcher/SettingPages/ViewModels/DropdownDataGeneric.cs (2)
21-22: Updated translation retrieval method in GetValues.Modified to use App.API.GetTranslation instead of the previous InternationalizationManager.Instance approach, aligning with the PR's goal of improving dependency management.
32-33: Updated translation retrieval method in UpdateLabels.Consistently switched to App.API.GetTranslation for retrieving translations in the UpdateLabels method as well, maintaining the code improvements across the class.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneThemeViewModel.cs (5)
10-10: Dependency Injection import added.This change introduces the dependency injection capability, allowing for the use of
Ioc.Default.GetRequiredService<Theme>().
26-26: Switched from singleton to DI-based Theme instance.Excellent change that moves away from the singleton pattern (
ThemeManager.Instance) to a properly injected service, which improves testability and reduces global state.
38-38: Theme instance accessor changed.All references to
ThemeManager.Instancehave been properly replaced with the injected_themeinstance, ensuring consistent use of dependency injection throughout the class.Also applies to: 40-40, 50-50, 58-58, 62-62, 100-100, 303-303, 325-325, 347-347, 369-369, 393-393, 414-414
52-52: API message display standardized.Changed from using
Notification.Showto the standardizedApp.API.ShowMsgBoxmethod, which provides a more consistent API surface.
243-244: Translation calls standardized.Replaced direct calls to
InternationalizationManager.Instance.GetTranslationwith the API-based approachApp.API.GetTranslation, making internationalization more consistent throughout the codebase.Also applies to: 252-253, 261-262, 270-271
Flow.Launcher/ActionKeywords.xaml.cs (1)
43-43: Translation call standardized.Replaced
translater.GetTranslationwithApp.API.GetTranslation, aligning with the PR objective to centralize translation access.Flow.Launcher/SettingPages/ViewModels/SettingsPaneAboutViewModel.cs (3)
25-25: Translation calls standardized.All calls to
InternationalizationManager.Instance.GetTranslationhave been properly replaced withApp.API.GetTranslation, providing a more consistent approach to internationalization.Also applies to: 43-44, 64-66
78-78: Directory access standardized.Changed direct directory operations to use the centralized API method
App.API.OpenDirectory, improving code consistency.Also applies to: 86-86
97-97: Method naming improved for async method.Renamed
UpdateApptoUpdateAppAsyncto better follow naming conventions for asynchronous methods, making the code more maintainable.Flow.Launcher/MainWindow.xaml.cs (2)
349-354: Translation calls standardized in menu and tooltips.All menu item headers and tooltips now use
App.API.GetTranslationinstead of direct internationalization manager calls, providing a more consistent API surface.Also applies to: 376-376, 381-381, 387-387, 393-393, 402-403
438-438: Settings save standardized.Changed from direct settings save to using the centralized API method
App.API.SaveAppAllSettings, improving code consistency.Flow.Launcher/CustomShortcutSetting.xaml.cs (3)
10-11: String.Empty changed to string.Empty for consistency.This change follows C# convention by using lowercase
stringinstead of the class nameStringfor consistency in the codebase.
41-43: Properly replaced direct translation call with API function.The code now uses
App.API.ShowMsgBoxwithApp.API.GetTranslationinstead of direct calls to InternationalizationManager, which aligns with the PR objective of improved dependency injection.
49-50: Translation and notification calls consolidated through the API.Consistent with the other change in this file, this properly replaces direct calls to InternationalizationManager with API functions.
Plugins/Flow.Launcher.Plugin.Explorer/Search/ResultManager.cs (2)
166-166: Plugin translation call properly updated to use Context.API.Changed from using
InternationalizationManager.Instance.GetTranslationtoContext.API.GetTranslation, which aligns with the PR objective while using the plugin's appropriate context object.
321-321: Updated tooltip translation to use Context.API consistently.This change follows the same pattern as the previous translation update in this file, maintaining consistency in how translations are retrieved throughout the plugin.
Flow.Launcher.Core/Plugin/PluginManager.cs (3)
32-32: Improved API encapsulation with dependency injection.Making the API private and using dependency injection is the core change of this PR. This implementation correctly retrieves the API instance through the IoC container rather than having it set externally, which improves encapsulation and testability.
66-66: Updated all API references to use the private field.All references to the previously public API property have been consistently replaced with the private
_apifield throughout the file. This maintains the functionality while improving encapsulation.Also applies to: 171-171, 213-216, 523-523, 558-559, 572-573, 589-590
206-208: Improved internationalization with proper DI pattern.The code now correctly uses dependency injection to get the
Internationalizationservice rather than accessing a singleton directly. This is a significant improvement in the architecture that aligns with modern dependency injection practices.Flow.Launcher/HotkeyControlDialog.xaml.cs (2)
34-34: Replaced static translation with App.API call.The static property now correctly uses the API for translation instead of accessing the InternationalizationManager directly, aligning with the PR objectives.
42-43: Consistently updated all translation calls to use App.API.All instances of
InternationalizationManager.Instance.GetTranslationhave been replaced withApp.API.GetTranslation, maintaining a consistent approach throughout the class.Also applies to: 142-144, 149-151, 161-163, 177-178
Flow.Launcher/App.xaml.cs (3)
94-94: Dependency injection for internationalization: Good change.This change replaces a direct call to
InternationalizationManager.Instancewith dependency injection usingIoc.Default.GetRequiredService<Internationalization>(), improving code maintainability by reducing reliance on singletons.
114-114: Dependency injection for theme management: Good change.Similar to the internationalization change, this replaces a direct call to
ThemeManager.Instancewith dependency injection, making the code more modular and testable.
144-144: Consolidated message display through API: Good change.Replaced direct calls to notification and translation systems with a consolidated API approach using
API.ShowMsgandAPI.GetTranslation, improving consistency and maintainability.Flow.Launcher/PublicAPIInstance.cs (4)
37-37: Added translation service field: Good change.Adding a private field for the translation service that will be injected through the constructor.
42-42: Constructor dependency injection: Good change.Constructor now accepts
Internationalizationparameter and properly stores it in the_translaterfield, following dependency injection best practices.Also applies to: 45-46
158-158: Translation method implementation: Good change.Replaced direct call to
InternationalizationManager.Instancewith the injected_translaterservice, reducing tight coupling and improving testability.
165-165: Added cancellation token support: Good enhancement.HTTP methods now include optional
CancellationTokenparameters, allowing for better control over asynchronous operations and potential cancellation.Also applies to: 167-168
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (5)
4-4: Added dependency injection namespace: Good change.Added the namespace for
DependencyInjectionto support retrieving services from the IoC container.
51-51: Using API for notifications and translations: Good change.Replaced direct call to
Notification.ShowwithApp.API.ShowMsgand usingApp.API.GetTranslationfor localization, improving consistency with the rest of the codebase.
118-120: Using injected translation service: Good change.Replaced calls to
InternationalizationManager.Instancewith the_translaterservice for language changes and Pinyin prompts, reducing dependency on singletons.
133-134: Service retrieval and language list access: Good change.Added field initialization for the translation service using dependency injection and updated
Languagesproperty to use this service instead of the singleton.
137-138: Consistent translation approach: Good change.All translation string retrievals now use
App.API.GetTranslationinstead of direct calls to the internationalization manager, ensuring consistency throughout the codebase.Also applies to: 183-184, 195-196
Flow.Launcher/ViewModel/MainViewModel.cs (8)
251-252: Approve: Translation API usage for reload plugin messageThe refactoring here properly replaces the direct call to InternationalizationManager with App.API.GetTranslation() and uses App.API.ShowMsg() for displaying notifications, which matches the PR objectives.
442-442: Approve: API usage for opening URLGood refactoring to use App.API.OpenUrl() which centralizes external URL handling through the API.
1045-1046: Approve: Translation API usage in history queryThe refactoring correctly replaces InternationalizationManager references with App.API.GetTranslation() for history-related text.
1285-1285: Approve: Translation API usage for context menuGood replacement of direct InternationalizationManager call with App.API.GetTranslation() for the context menu title.
1291-1291: Approve: ShowMsg API usageThe refactoring properly uses App.API.ShowMsg() with App.API.GetTranslation() instead of direct calls to Notification and InternationalizationManager.
1301-1301: Approve: Translation API usage for context menuSuccessfully replaces InternationalizationManager reference with App.API.GetTranslation() for the "set as top most" option.
1308-1308: Approve: ShowMsg API usage for success messageGood replacement of direct notification call with App.API.ShowMsg() that uses the translation API.
1322-1325: Approve: Translation API usage for plugin metadataAll plugin info translations are correctly migrated from InternationalizationManager to App.API.GetTranslation().
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: Yusyuriv, jjw24 Jack251970, jjw24 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher.Core/Plugin/PluginManager.cs (1)
32-34: Variable naming convention inconsistencyThe field
apion line 33 is using camelCase while other fields like_api(referenced throughout the file) use underscore prefix. This seems inconsistent with the codebase style.Consider using consistent naming:
- private static IPublicAPI api = null; - private static IPublicAPI API => api ??= Ioc.Default.GetRequiredService<IPublicAPI>(); + private static IPublicAPI _api = null; + private static IPublicAPI API => _api ??= Ioc.Default.GetRequiredService<IPublicAPI>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Core/Plugin/PluginManager.cs(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (8)
Flow.Launcher.Core/Plugin/PluginManager.cs (8)
68-68: Good refactoring: using encapsulated API accessThis correctly uses the private API instance after the refactoring to encapsulate the API.
173-173: Good refactoring: using encapsulated API in plugin initializationThis correctly uses the private API instance in plugin initialization context.
208-210: Improved dependency management using IoCThe code now properly uses dependency injection to retrieve services instead of directly accessing singleton instances. This is aligned with the PR objectives to remove InternationalizationManager.Instance.
215-219: Good refactoring: using API methods for translation and notificationsThe changes properly implement the PR objective to use API functions instead of direct manager access.
525-525: Good refactoring: using encapsulated API for message displayThis correctly uses the encapsulated API for showing messages.
560-562: Good refactoring: using encapsulated API for reflectionThe code properly uses the private API instance for reflection operations.
574-575: Good refactoring: using API methods for error messagesThe changes properly implement the PR objective to use API functions instead of direct notification access.
591-592: Good refactoring: using API methods for error messagesThe changes properly implement the PR objective to use API functions instead of direct notification access.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)If the flagged items are 🤯 false positivesIf items relate to a ...
|
Improve code quality
App.API.GetTranslationinsteadApp.API.ShowMsginstead