New API Function LoadImageAsync #3412
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new API function, LoadImageAsync, to centralize and standardize image loading across plugins and the core application while also improving error logging and overall code quality.
- Replaces direct calls to ImageLoader.LoadAsync with API.LoadImageAsync.
- Updates HTTP and logging calls to use centralized API methods.
- Augments ImageLoader to support an optional caching flag for improved performance.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs | Updated API and logging usage for Google suggestions. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs | Updated API and logging usage for DuckDuckGo suggestions. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs | Updated API and logging usage, with a noted error message issue. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs | Updated API and logging usage for Baidu suggestions. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SearchSourceViewModel.cs | Replaced ImageLoader.LoadAsync with API.LoadImageAsync for image preview. |
| Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs | Updated preview loading to use the new API call. |
| Flow.Launcher/ViewModel/ResultViewModel.cs | Centralized image loading via the new API function. |
| Flow.Launcher/ViewModel/PluginViewModel.cs | Updated plugin icon loading through the API. |
| Flow.Launcher/ViewModel/MainViewModel.cs | Switched to using the API for plugin icon retrieval. |
| Flow.Launcher/PublicAPIInstance.cs | Added the new LoadImageAsync API method. |
| Flow.Launcher/Msg.xaml.cs | Updated image loading in dialog views to use the API. |
| Flow.Launcher/MessageBoxEx.xaml.cs | Replaced direct image loading with the new API call. |
| Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs | Extended the interface to include LoadImageAsync with detailed comment documentation. |
| Flow.Launcher.Infrastructure/Image/ImageLoader.cs | Modified LoadAsync to include caching support. |
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
🥷 Code experts: no user but you matched threshold 10 Jack251970 has 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: To learn more about /:\ gitStream - Visit our Docs |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughThe pull request introduces modifications to the image loading functionality and HTTP/logging mechanisms across the project. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant ImageLoader
Client->>API: LoadImageAsync(path, loadFullImage, cacheImage)
API->>ImageLoader: LoadAsync(path, loadFullImage, cacheImage)
ImageLoader-->>API: Return ImageSource (with caching as needed)
API-->>Client: Deliver ImageSource
sequenceDiagram
participant SuggestionSource
participant API
participant HTTPService
SuggestionSource->>API: Request HTTP data (via HttpGetStreamAsync/HttpGetStringAsync)
API->>HTTPService: Make HTTP request
HTTPService-->>API: Return HTTP response
API-->>SuggestionSource: Forward response data
alt Error Occurs
SuggestionSource->>API: LogException(error)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
🪧 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: 1
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs (1)
36-36: Fix log message to correctly reference BingThe log message incorrectly references "baidu" instead of "Bing" which is inconsistent with the class name.
- Main._context.API.LogException(nameof(Bing), "Can't get suggestion from baidu", e); + Main._context.API.LogException(nameof(Bing), "Can't get suggestion from Bing", e);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs(2 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs(2 hunks)Flow.Launcher/MessageBoxEx.xaml.cs(1 hunks)Flow.Launcher/Msg.xaml.cs(2 hunks)Flow.Launcher/PublicAPIInstance.cs(2 hunks)Flow.Launcher/ViewModel/MainViewModel.cs(1 hunks)Flow.Launcher/ViewModel/PluginViewModel.cs(1 hunks)Flow.Launcher/ViewModel/ResultViewModel.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs(1 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SearchSourceViewModel.cs(3 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs(2 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs(3 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs(2 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
Flow.Launcher/ViewModel/MainViewModel.cs (1)
Flow.Launcher/App.xaml.cs (2)
App(28-343)App(50-114)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs (2)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
ValueTask(362-362)Flow.Launcher/PublicAPIInstance.cs (1)
ValueTask(346-347)
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs (3)
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs (1)
Task(13-38)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs (1)
Task(13-45)Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs (1)
Task(16-52)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (25)
Flow.Launcher/MessageBoxEx.xaml.cs (1)
159-159: API change improves consistency and plugin accessibilityThe code now uses
App.API.LoadImageAsyncwhich aligns with the PR objective to unify all image loading calls through the API layer.Flow.Launcher/ViewModel/MainViewModel.cs (1)
1171-1171: API call replacement is aligned with project changesThis change replaces direct usage of
ImageLoader.LoadAsyncwith the new API interface methodApp.API.LoadImageAsync, consistent with the PR's objective to centralize image loading functionality.Flow.Launcher/ViewModel/ResultViewModel.cs (1)
202-202: Image loading functionality unified through API layerThe implementation now properly uses
App.API.LoadImageAsyncinstead of directly accessingImageLoader.LoadAsync, maintaining the optionalloadFullImageparameter. This change improves plugin extensibility by providing a unified API for image loading.Flow.Launcher/ViewModel/PluginViewModel.cs (1)
48-48: Consistent API migration for plugin icon loadingThe change to use
App.API.LoadImageAsyncis consistent with similar changes throughout the codebase, enhancing maintainability and providing a unified approach to image loading.Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs (1)
91-91: Clean API transition for image loading!The code now uses
Main.Context.API.LoadImageAsyncinstead of directly calling the ImageLoader, which aligns with the PR's goal to unify image loading through the API layer.Flow.Launcher/Msg.xaml.cs (3)
46-46: Good refactoring to use the new APIThe code correctly uses the new
App.API.LoadImageAsyncmethod instead of directly callingImageLoader.LoadAsync, aligning with the PR's goal of centralizing image loading through the API.
74-75: Appropriate API usage for image loadingThe close image loading is now properly handled through the API layer rather than directly accessing the ImageLoader.
78-79: Consistent API usage throughout the methodThis change maintains consistency with the other image loading operations in this method, following the same pattern.
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs (2)
19-19: Good API transition for HTTP requestsThe code now uses the API context for HTTP requests instead of direct calls to the Http utility, which improves maintainability.
35-35: Consistent API usage for loggingGood practice to use the API context for logging exceptions instead of directly accessing the Log utility.
Flow.Launcher/PublicAPIInstance.cs (2)
13-13: Necessary namespace additionThe System.Windows.Media namespace is correctly added to support the ImageSource return type of the new LoadImageAsync method.
346-348: Well-implemented new API methodThe new LoadImageAsync method is correctly implemented with appropriate default parameters. This is the core of the PR and enables plugins to load images from various sources with an option to control caching.
loadFullImage = falseensures optimization by defaultcacheImage = trueenables performance improvements by default- The method simply delegates to the existing ImageLoader functionality
This implementation achieves the PR's main objective of exposing image loading functionality to plugins.
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs (2)
26-26: LGTM: Great use of the API for HTTP requestsThis change properly uses the centralized API interface method for HTTP requests instead of direct calls, which aligns with the PR objective of encouraging API usage in the WebSearch plugin.
37-37: LGTM: Consistent error logging via APIGood job updating the exception logging to use the API interface, maintaining a consistent approach across the codebase.
Also applies to: 42-42
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
349-362: LGTM: Well-documented LoadImageAsync API additionThe new
LoadImageAsyncmethod is well-documented with clear XML comments explaining the parameters and their purposes. This addition successfully exposes the image loading functionality to all plugins as intended by the PR.Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs (2)
23-23: LGTM: Good usage of API for HTTP requestsThis change properly utilizes the centralized API interface method for HTTP string requests, aligning with the PR's goal of improving code quality in the WebSearch plugin.
27-27: LGTM: Consistent error handling approachGood job updating the exception logging to use the API interface consistently with other suggestion sources.
Also applies to: 42-42
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs (3)
11-11: LGTM: Appropriate visibility changeMaking the Bing class public improves consistency with other suggestion source classes.
19-19: LGTM: Consistent API usage for HTTP requestsGood job utilizing the centralized API interface method for HTTP stream requests, aligning with similar changes in other suggestion sources.
41-41: LGTM: Consistent error handlingThe exception logging has been properly updated to use the API interface.
Plugins/Flow.Launcher.Plugin.WebSearch/SearchSourceViewModel.cs (2)
43-44: LGTM: Consistent error handling using API.The change ensures consistent use of the API's ShowMsgBox method for error handling, which aligns with the best practice of using API functions instead of direct project calls.
63-63: Great API integration!This change improves code quality by replacing a direct call to
ImageLoader.LoadAsyncwith the new API methodLoadImageAsync, perfectly aligning with the PR objective to unify all image loading functionality under the API.Flow.Launcher.Infrastructure/Image/ImageLoader.cs (3)
280-280: Well-designed method signature enhancement.The addition of the optional
cacheImageparameter with a default value oftruemaintains backward compatibility while adding new functionality, allowing plugins to control whether images are cached.
296-301: Good implementation of conditional GUID caching.The conditional saving of the GUID key based on the
cacheImageparameter is well implemented, allowing for selective caching of image hash references.
303-307: Excellent implementation of conditional image caching.The conditional updating of the image cache based on the
cacheImageparameter provides precise control over caching behavior, which can significantly improve performance for plugins that don't need persistent image caching.
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces a new API function, LoadImageAsync, to centralize image loading across plugins and improve consistency when handling local, remote, or data:image URLs with an optional caching mechanism. Key changes include replacing direct calls to ImageLoader.LoadAsync with API invocations, updating logging calls to use the new API, and modifying the associated interface and documentation.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs | Replaces direct HTTP stream and logging calls with API-based equivalents. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs | Applies similar modifications replacing direct calls with API-based calls. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs | Adjusts class visibility and updates image fetching and logging to use the API. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs | Refactors image fetching and logging to leverage the API and aligns logging error messages. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SearchSourceViewModel.cs | Updates image loading to use the API in place of direct ImageLoader calls. |
| Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs | Migrates image loading in the preview panel to the new API method. |
| Flow.Launcher/ViewModel/ResultViewModel.cs | Changes image fetching to use App.API.LoadImageAsync for consistency. |
| Flow.Launcher/ViewModel/PluginViewModel.cs | Updates plugin icon loading to use the new API method. |
| Flow.Launcher/ViewModel/MainViewModel.cs | Refactors the logic for loading the plugin icon to use API calls. |
| Flow.Launcher/PublicAPIInstance.cs | Adds the new LoadImageAsync API method to the PublicAPIInstance implementation. |
| Flow.Launcher/Msg.xaml.cs | Updates image loading for UI elements to use the API in place of ImageLoader. |
| Flow.Launcher/MessageBoxEx.xaml.cs | Refactors image fetching for message box icons to use the new API method. |
| Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs | Extends the interface to include the LoadImageAsync method with parameters for full image and cache. |
| Flow.Launcher.Infrastructure/Image/ImageLoader.cs | Modifies the LoadAsync implementation to accept a cacheImage flag and adjust caching behavior. |
Comments suppressed due to low confidence (2)
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs:27
- [nitpick] Consider capitalizing 'baidu' to 'Baidu' in the error message for consistency with other plugins.
Main._context.API.LogException(nameof(Baidu), "Can't get suggestion from baidu", e);
Flow.Launcher.Infrastructure/Image/ImageLoader.cs:296
- Review the conditional logic: when cacheImage is false, the GUID-to-key mapping is skipped—ensure this behavior is intentional.
else if (cacheImage)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new API function, LoadImageAsync, to unify and simplify image loading across various plugins and parts of the project. Key changes include:
- Replacing direct calls to ImageLoader.LoadAsync with API-based LoadImageAsync.
- Updating error logging to use API.LogException.
- Adding new API endpoints in PublicAPIInstance and IPublicAPI interfaces to support image loading from local, remote, or data URLs.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs | Replaced direct HTTP calls and logging with API method calls. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/DuckDuckGo.cs | Updated HTTP requests and exception logging to use the new API. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Bing.cs | Changed class scope and API calls for image and suggestion loading. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Baidu.cs | Updated HTTP and logging calls to use API methods. |
| Plugins/Flow.Launcher.Plugin.WebSearch/SearchSourceViewModel.cs | Modified image preview loading to use API.LoadImageAsync. |
| Plugins/Flow.Launcher.Plugin.Explorer/Views/PreviewPanel.xaml.cs | Updated image loading to utilize API instead of direct ImageLoader calls. |
| Flow.Launcher/ViewModel/ResultViewModel.cs | Replaced image loading to use App.API.LoadImageAsync. |
| Flow.Launcher/ViewModel/PluginViewModel.cs | Updated icon loading with new API function. |
| Flow.Launcher/ViewModel/MainViewModel.cs | Changed image loading to use App.API.LoadImageAsync. |
| Flow.Launcher/PublicAPIInstance.cs | Added new API method LoadImageAsync. |
| Flow.Launcher/Msg.xaml.cs | Updated image loading calls to use App.API.LoadImageAsync. |
| Flow.Launcher/MessageBoxEx.xaml.cs | Changed image loading to use App.API.LoadImageAsync. |
| Flow.Launcher/Plugin/Interfaces/IPublicAPI.cs | Added LoadImageAsync method signature with caching options. |
| Flow.Launcher.Infrastructure/Image/ImageLoader.cs | Modified LoadAsync to accept a cacheImage parameter and updated caching logic accordingly. |
Comments suppressed due to low confidence (2)
Plugins/Flow.Launcher.Plugin.WebSearch/SuggestionSources/Google.cs:19
- [nitpick] Consider using a consistent API context reference across the codebase (e.g., Main._context.API vs. Main.Context.API vs. App.API) to improve readability and maintainability.
await using var resultStream = await Main._context.API.HttpGetStreamAsync(api + Uri.EscapeDataString(query), token: token).ConfigureAwait(false);
Flow.Launcher.Infrastructure/Image/ImageLoader.cs:296
- [nitpick] When caching is disabled (cacheImage = false), GuidToKey is not updated; if this behavior is intentional, please add a comment explaining why the mapping is skipped, to avoid confusion during maintenance.
else if (cacheImage)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
Flow.Launcher.Infrastructure/Image/ImageLoader.cs:303
- When caching is disabled (cacheImage is false), the GuidToKey mapping is not updated, which may cause inconsistencies if this mapping is subsequently used. Please verify that this behavior is intentional and consider adding tests to ensure that image identification remains correct when caching is turned off.
else if (cacheImage)
This comment has been minimized.
This comment has been minimized.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
New API Function
LoadImageAsyncExpose
ImageLoader.LoadAsyncto api function so that all plugins can load images from local, remove or data:image url. They can enablecacheImageoption to let FL cache those images to improve performance.Unify all calls of
ImageLoader.LoadAsynctoAPI.LoadImageAsyncImprove
WebSearchplugin code qualityUse api functions instead of direct project calls in this plugin.