Skip to content

Conversation

Jack251970
Copy link
Member

@Jack251970 Jack251970 commented Apr 4, 2025

New API functions

Add api functions from PluginManifest and PluginManager. This can help PluginManager drop reference to FL project and also make third-party plugin manager plugin possible.

/// <summary>
/// Update the plugin manifest
/// </summary>
/// <param name="usePrimaryUrlOnly">
/// FL has multiple urls to download the plugin manifest. Set this to true to only use the primary url.
/// </param>
/// <param name="token"></param>
/// <returns>True if the manifest is updated successfully, false otherwise</returns>
public Task<bool> UpdatePluginManifestAsync(bool usePrimaryUrlOnly = false, CancellationToken token = default);

/// <summary>
/// Get the plugin manifest
/// </summary>
/// <returns></returns>
public IReadOnlyList<UserPlugin> GetPluginManifest();

/// <summary>
/// Check if the plugin has been modified.
/// If this plugin is updated, installed or uninstalled and users do not restart the app,
/// it will be marked as modified
/// </summary>
/// <param name="id">Plugin id</param>
/// <returns></returns>
public bool PluginModified(string id);

/// <summary>
/// Update a plugin to new version, from a zip file. By default will remove the zip file if update is via url,
/// unless it's a local path installation
/// </summary>
/// <param name="pluginMetadata">The metadata of the old plugin to update</param>
/// <param name="plugin">The new plugin to update</param>
/// <param name="zipFilePath">
/// Path to the zip file containing the plugin. It will be unzipped to the temporary directory, removed and installed.
/// </param>
/// <returns></returns>
public Task UpdatePluginAsync(PluginMetadata pluginMetadata, UserPlugin plugin, string zipFilePath);

/// <summary>
/// Install a plugin. By default will remove the zip file if installation is from url,
/// unless it's a local path installation
/// </summary>
/// <param name="plugin">The plugin to install</param>
/// <param name="zipFilePath">
/// Path to the zip file containing the plugin. It will be unzipped to the temporary directory, removed and installed.
/// </param>
public void InstallPlugin(UserPlugin plugin, string zipFilePath);

/// <summary>
/// Uninstall a plugin
/// </summary>
/// <param name="pluginMetadata">The metadata of the plugin to uninstall</param>
/// <param name="removePluginSettings">
/// Plugin has their own settings. If this is set to true, the plugin settings will be removed.
/// </param>
/// <returns></returns>
public Task UninstallPluginAsync(PluginMetadata pluginMetadata, bool removePluginSettings = false);

This comment has been minimized.

Copy link

gitstream-cm bot commented Apr 4, 2025

🥷 Code experts: jjw24

jjw24 has most 🧠 knowledge in the files.

See details

Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs

Knowledge based on git-blame:

Flow.Launcher.Core/ExternalPlugins/CommunityPluginStore.cs

Knowledge based on git-blame:

Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs

Knowledge based on git-blame:

Flow.Launcher.Core/Plugin/PluginManager.cs

Knowledge based on git-blame:
jjw24: 3%

Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs

Knowledge based on git-blame:
jjw24: 17%

Flow.Launcher/PublicAPIInstance.cs

Knowledge based on git-blame:
jjw24: 7%

Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs

Knowledge based on git-blame:

Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs

Knowledge based on git-blame:

Plugins/Flow.Launcher.Plugin.PluginsManager/ContextMenu.cs

Knowledge based on git-blame:
jjw24: 78%

Plugins/Flow.Launcher.Plugin.PluginsManager/Flow.Launcher.Plugin.PluginsManager.csproj

Knowledge based on git-blame:
jjw24: 85%

Plugins/Flow.Launcher.Plugin.PluginsManager/Main.cs

Knowledge based on git-blame:
jjw24: 56%

Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs

Knowledge based on git-blame:
jjw24: 15%

Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs

Knowledge based on git-blame:
jjw24: 70%

To learn more about /:\ gitStream - Visit our Docs

Copy link

gitstream-cm bot commented Apr 4, 2025

Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX.

Copy link
Contributor

coderabbitai bot commented Apr 4, 2025

📝 Walkthrough

Walkthrough

This pull request introduces several enhancements and refactorings related to plugin management in Flow Launcher. It includes the addition of new using directives, updates to namespace references, and modifications to method signatures and variable declarations. Key changes involve the introduction of new API methods for managing plugins, including installation, updating, and uninstallation. Calls to legacy plugin management methods have been replaced with updated API calls, reflecting a transition to a more centralized approach for plugin operations across the application.

Changes

File(s) Change Summary
Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs
Flow.Launcher.Core/ExternalPlugins/CommunityPluginStore.cs
Added using directive: using Flow.Launcher.Plugin;
Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs Removed Flow.Launcher.Infrastructure.Logger; added CommunityToolkit.Mvvm.DependencyInjection and Flow.Launcher.Plugin namespaces; changed fetchTimeout to readonly; updated parameter order and error logging in UpdateManifestAsync
Flow.Launcher.Core/ExternalPlugins/UserPlugin.cs
Flow.Launcher.Plugin/UserPlugin.cs
Removed the UserPlugin record from Core and introduced a new UserPlugin record in the Plugin namespace with updated properties and XML comments
Flow.Launcher.Core/Plugin/PluginManager.cs Renamed method parameter from uuid to id; removed commented XML docs; reformatted defaultPluginIDs
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
Flow.Launcher/PublicAPIInstance.cs
Added new plugin management methods: UpdatePluginManifestAsync, GetPluginManifest, PluginModified, UpdatePluginAsync, InstallPlugin, and UninstallPluginAsync
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs Updated external plugin retrieval and refresh calls to use App.API instead of PluginsManifest
Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
Plugins/Flow.Launcher.Plugin.PluginsManager/ContextMenu.cs
Removed unused using directives (including Flow.Launcher.Core.ExternalPlugins and SemanticVersioning)
Plugins/Flow.Launcher.Plugin.PluginsManager/Flow.Launcher.Plugin.PluginsManager.csproj
Plugins/Flow.Launcher.Plugin.PluginsManager/Main.cs
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs
Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs
Removed unneeded project references; added a package reference to SharpZipLib (v1.4.2); updated the Context property to static; refined API calls (switching to Context.API) and renamed constants; removed unused using directives

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant VM as SettingsPanePluginStoreViewModel
    participant API as PublicAPIInstance
    participant PM as PluginManager

    U->>VM: Initiate plugin update
    VM->>API: UpdatePluginManifestAsync(usePrimaryUrlOnly)
    API->>PM: Forward request via Context.API
    PM-->>API: Return update result
    API-->>VM: Return updated plugin manifest
Loading

Suggested labels

enhancement

Suggested reviewers

  • taooceros
  • jjw24
  • VictoriousRaptor

Poem

I’m a rabbit with a jubilant cheer,
Hopping through code with revisions so clear.
New APIs and changes hopping in sight,
Refactored pathways make our code just right.
🐇 Let our plugins dance in the light,
With every update, our dreams take flight!
~ Happy coding, from a rabbit so bright!

✨ Finishing Touches
  • 📝 Generate Docstrings

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (5)
Flow.Launcher.Plugin/UserPlugin.cs (3)

8-13: Consider using nullable reference types for string properties.

Since C# 8.0, it's recommended to use nullable reference types to avoid null reference exceptions. For a model class that will be deserialized from JSON, it's important to handle potential null values.

/// <summary>
/// Unique identifier of the plugin
/// </summary>
-public string ID { get; set; }
+public string? ID { get; set; }

Consider adding the nullable annotation context to the file:

using System;
+#nullable enable

namespace Flow.Launcher.Plugin

8-79: Consider adding validation for required properties.

For a plugin model, certain properties like ID and Name are likely required. Consider adding validation to ensure these properties are not null or empty, either through a constructor, initialization, or validation method.

You could add a validation method like:

public bool Validate()
{
    return !string.IsNullOrEmpty(ID) && 
           !string.IsNullOrEmpty(Name) && 
           !string.IsNullOrEmpty(Version);
}

Or you could initialize properties with empty strings in a constructor:

public UserPlugin()
{
    ID = string.Empty;
    Name = string.Empty;
    Description = string.Empty;
    Author = string.Empty;
    Version = string.Empty;
    Language = string.Empty;
    Website = string.Empty;
    UrlDownload = string.Empty;
    UrlSourceCode = string.Empty;
    LocalInstallPath = string.Empty;
    IcoPath = string.Empty;
}

35-38: Improve the XML documentation reference.

The <see cref="AllowedLanguage"/> reference in the Language property documentation doesn't appear to refer to a visible type in this file. Ensure that this reference is correct and accessible, or provide more context about the allowed languages.

/// <summary>
-/// Allow language of the plugin <see cref="AllowedLanguage"/>
+/// Language of the plugin (e.g., C#, Python, etc.)
/// </summary>
public string Language { get; set; }

Alternatively, if AllowedLanguage is defined elsewhere:

/// <summary>
/// Allow language of the plugin <see cref="AllowedLanguage"/>
/// </summary>
public string Language { get; set; }
Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (2)

15-21: Consider adding null-handling logic

While you've correctly used the null-conditional operator (?.) with GetPluginManifest(), consider adding explicit handling in case the manifest is null - perhaps returning an empty list instead of null to prevent potential null reference exceptions further down the chain.

public IList<PluginStoreItemViewModel> ExternalPlugins => 
-    App.API.GetPluginManifest()?.Select(p => new PluginStoreItemViewModel(p))
+    App.API.GetPluginManifest()?.Select(p => new PluginStoreItemViewModel(p))
     .OrderByDescending(p => p.Category == PluginStoreItemViewModel.NewRelease)
     .ThenByDescending(p => p.Category == PluginStoreItemViewModel.RecentlyUpdated)
     .ThenByDescending(p => p.Category == PluginStoreItemViewModel.None)
     .ThenByDescending(p => p.Category == PluginStoreItemViewModel.Installed)
-    .ToList();
+    .ToList() ?? new List<PluginStoreItemViewModel>();

23-30: Consider adding error handling

The RefreshExternalPluginsAsync method doesn't include any error handling. If UpdatePluginManifestAsync throws an exception, it would propagate up the call stack. Consider adding a try-catch block to handle potential exceptions gracefully, especially since this is an async operation that might involve network requests.

[RelayCommand]
private async Task RefreshExternalPluginsAsync()
{
+    try
+    {
        if (await App.API.UpdatePluginManifestAsync())
        {
            OnPropertyChanged(nameof(ExternalPlugins));
        }
+    }
+    catch (System.Exception ex)
+    {
+        // Log the error or show a message to the user
+        Log.Exception($"Failed to refresh external plugins", ex);
+    }
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 67cc1e2 and 473b139.

📒 Files selected for processing (15)
  • Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs (1 hunks)
  • Flow.Launcher.Core/ExternalPlugins/CommunityPluginStore.cs (1 hunks)
  • Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (3 hunks)
  • Flow.Launcher.Core/ExternalPlugins/UserPlugin.cs (0 hunks)
  • Flow.Launcher.Core/Plugin/PluginManager.cs (2 hunks)
  • Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1 hunks)
  • Flow.Launcher.Plugin/UserPlugin.cs (1 hunks)
  • Flow.Launcher/PublicAPIInstance.cs (2 hunks)
  • Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (2 hunks)
  • Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs (0 hunks)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/ContextMenu.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/Flow.Launcher.Plugin.PluginsManager.csproj (1 hunks)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/Main.cs (3 hunks)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (20 hunks)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1 hunks)
💤 Files with no reviewable changes (2)
  • Flow.Launcher/ViewModel/PluginStoreItemViewModel.cs
  • Flow.Launcher.Core/ExternalPlugins/UserPlugin.cs
🧰 Additional context used
🧬 Code Definitions (4)
Flow.Launcher.Core/Plugin/PluginManager.cs (2)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
  • PluginModified (371-371)
Flow.Launcher/PublicAPIInstance.cs (1)
  • PluginModified (363-363)
Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (3)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (5)
  • LogException (234-234)
  • PluginModified (371-371)
  • InstallPlugin (393-393)
  • ShowMsgError (84-84)
  • GetTranslation (135-135)
Flow.Launcher/PublicAPIInstance.cs (5)
  • LogException (203-204)
  • PluginModified (363-363)
  • InstallPlugin (368-369)
  • ShowMsgError (109-110)
  • GetTranslation (171-171)
Flow.Launcher.Core/Plugin/PluginManager.cs (3)
  • PluginModified (457-460)
  • InstallPlugin (469-472)
  • InstallPlugin (483-555)
Flow.Launcher/PublicAPIInstance.cs (2)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (5)
  • Task (72-72)
  • Task (169-169)
  • IReadOnlyList (362-362)
  • PluginModified (371-371)
  • InstallPlugin (393-393)
Flow.Launcher.Core/Plugin/PluginManager.cs (5)
  • PluginModified (457-460)
  • PluginManager (23-627)
  • PluginManager (143-149)
  • InstallPlugin (469-472)
  • InstallPlugin (483-555)
Flow.Launcher.Plugin/UserPlugin.cs (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)
  • UserPlugin (62-80)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: gitStream.cm
🔇 Additional comments (41)
Flow.Launcher.Core/ExternalPlugins/CommunityPluginStore.cs (1)

5-5: Import addition supports centralized plugin management.

The addition of the Flow.Launcher.Plugin namespace import aligns with the PR's objective of refactoring plugin management functionality. This allows CommunityPluginStore to directly access types from the Plugin namespace, specifically the UserPlugin class referenced in the code.

Flow.Launcher.Core/ExternalPlugins/CommunityPluginSource.cs (1)

3-3: Import addition supports centralized plugin management.

The addition of the Flow.Launcher.Plugin namespace import is consistent with the PR's objective of centralizing plugin management functionality. This allows direct access to the UserPlugin type that's being used in the FetchAsync method.

Plugins/Flow.Launcher.Plugin.PluginsManager/ContextMenu.cs (1)

1-1: Import removal is part of plugin management refactoring.

The removal of the Flow.Launcher.Core.ExternalPlugins import is appropriate since UserPlugin is now directly accessible from the current namespace (Flow.Launcher.Plugin.PluginsManager). This change aligns with the PR objective of centralizing plugin management functionality and reducing direct dependencies on the ExternalPlugins namespace.

Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)

1-1: Import removal is part of plugin management refactoring.

The removal of the Flow.Launcher.Core.ExternalPlugins import is consistent with the PR's objective of centralizing plugin management functionality. The UserPlugin type used in the GetPluginInfoFromZip method is now accessible from the current namespace.

Plugins/Flow.Launcher.Plugin.PluginsManager/Flow.Launcher.Plugin.PluginsManager.csproj (1)

38-41: Good addition of SharpZipLib dependency.

Adding the SharpZipLib package provides necessary ZIP file handling capabilities for the new plugin management functionality. This aligns with the PR objectives that include methods for installing, updating, and uninstalling plugins which would require ZIP file operations.

Flow.Launcher.Core/Plugin/PluginManager.cs (2)

457-460: Good parameter naming change.

Renaming the parameter from uuid to id is a positive change that makes the method signature more consistent with the API interface definition and other parameter names in the codebase.


516-530: Improved code formatting.

The default plugin IDs list has been reformatted with proper indentation and line breaks, which improves readability while maintaining the same functionality.

Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs (4)

5-6: Appropriate namespace additions.

Adding the required namespaces for dependency injection and plugin interfaces supports the updated implementation for exception logging and API access.


21-21: Good practice: Adding readonly modifier.

Adding the readonly modifier to the fetchTimeout field is a good practice as it prevents the value from being accidentally changed after initialization.


25-25: Parameter order improvement.

Reordering the parameters to place usePrimaryUrlOnly first is logical since it's the primary functional parameter, while token is more of an infrastructure concern. Both parameters have default values, so this change won't break existing code.


47-47: Improved exception logging with dependency injection.

Switching from direct Log.Exception calls to using the API through dependency injection centralizes logging and aligns with modern DI practices. This change is consistent with the architectural improvements introduced in this PR.

Plugins/Flow.Launcher.Plugin.PluginsManager/Main.cs (4)

6-7: Appropriate using directives for ViewModel and View references.

These added using directives are correctly added to support access to the ViewModels and Views within the PluginsManager namespace.


13-13: Changed Context property from instance to static to improve accessibility.

The property is now static, which allows access without requiring an instance reference. This change supports the broader API refactoring being introduced in this PR where Context needs to be accessed from various places.


36-36: Updated to use the new API interface for updating plugin manifest.

This change correctly replaces the direct call to PluginsManifest.UpdateManifestAsync() with the new API method Context.API.UpdatePluginManifestAsync(), which aligns with the PR objective of centralizing plugin management functions in the API.


57-57: Updated to use API-based fuzzy search.

This change correctly replaces the direct call to StringMatcher.FuzzySearch with Context.API.FuzzySearch, maintaining consistency with the API-based approach being used for other functionality.

Plugins/Flow.Launcher.Plugin.PluginsManager/PluginsManager.cs (11)

15-15: Renamed constant for improved code clarity.

Renaming zip to ZipSuffix provides better semantic meaning and follows proper naming conventions for constants.


17-17: Added class name constant for consistent logging.

Adding a static readonly string for the class name is a good practice for ensuring consistent logging throughout the class and avoiding string literals.


51-51: Updated to use C# target-typed new expressions.

The code now uses the shorthand new() syntax introduced in C# 9, which improves readability while maintaining type safety.

Also applies to: 63-63, 75-75, 276-276, 552-552


169-169: Standardized exception logging through the API.

The code now consistently uses Context.API.LogException(ClassName, message, exception) for all exception logging, which provides better centralized error handling and consistent logging format.

Also applies to: 179-179, 366-367, 438-438, 655-655, 662-662, 669-669, 743-743


237-237: Updated to use the new API method for updating plugin manifest.

This change correctly uses Context.API.UpdatePluginManifestAsync() instead of direct calls to PluginsManifest.UpdateManifestAsync(), aligning with the PR objective of centralizing plugin operations through the API.

Also applies to: 602-602


250-250: Updated to use API methods for getting plugin manifest and checking modifications.

The code now uses Context.API.GetPluginManifest() and Context.API.PluginModified(id) instead of directly accessing PluginsManifest.UserPlugins and plugin modification flags, which improves encapsulation and follows the new API design.

Also applies to: 612-613


341-342: Updated to use the new API method for updating plugins.

The code now uses Context.API.UpdatePluginAsync() to update plugins, which aligns with the PR's objective of centralizing plugin management through the API.

Also applies to: 433-434


646-646: Updated to use the new API method for installing plugins.

The code now uses Context.API.InstallPlugin() instead of directly calling PluginManager.InstallPlugin(), following the new API-centric approach for plugin management.


739-739: Updated to use the new API method for uninstalling plugins.

The code now uses Context.API.UninstallPluginAsync() for plugin uninstallation, which is consistent with the PR's objective of centralizing plugin operations through the API.


498-498: Updated string format to use named constant.

The code now uses the ZipSuffix constant in the string format operation instead of a string literal, improving maintainability and consistency.


605-605: Updated to use the named constant for file extension check.

The code now uses the ZipSuffix constant instead of a string literal when checking file extensions, ensuring consistency throughout the codebase.

Flow.Launcher/PublicAPIInstance.cs (7)

31-31: Added appropriate using directive for external plugins.

This import of Flow.Launcher.Core.ExternalPlugins is necessary to access the PluginsManifest class used in the new API methods.


358-359: Added new API method for updating plugin manifest.

This method correctly implements the UpdatePluginManifestAsync method declared in the IPublicAPI interface by delegating to the existing PluginsManifest.UpdateManifestAsync method. This addition aligns with the PR objectives.


361-361: Added new API method for retrieving plugin manifest.

This method correctly implements the GetPluginManifest method declared in the IPublicAPI interface by returning the existing PluginsManifest.UserPlugins property. This addition aligns with the PR objectives.


363-363: Added new API method for checking if a plugin has been modified.

This method correctly implements the PluginModified method declared in the IPublicAPI interface by delegating to the existing PluginManager.PluginModified method. This addition aligns with the PR objectives.


365-366: Added new API method for updating plugins.

This method correctly implements the UpdatePluginAsync method declared in the IPublicAPI interface by delegating to the existing PluginManager.UpdatePluginAsync method. This addition aligns with the PR objectives.


368-369: Added new API method for installing plugins.

This method correctly implements the InstallPlugin method declared in the IPublicAPI interface by delegating to the existing PluginManager.InstallPlugin method. This addition aligns with the PR objectives.


371-372: Added new API method for uninstalling plugins.

This method correctly implements the UninstallPluginAsync method declared in the IPublicAPI interface by delegating to the existing PluginManager.UninstallPluginAsync method. This addition aligns with the PR objectives.

Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (6)

348-356: Added well-documented API method for updating plugin manifest.

This method declaration includes comprehensive XML documentation that clearly explains its purpose, parameters, and return value. It properly specifies default parameter values and return type.


358-362: Added well-documented API method for retrieving plugin manifest.

This method declaration includes clear XML documentation that explains its purpose and return value. The return type IReadOnlyList<UserPlugin> is appropriate as it provides read-only access to the plugin list.


364-371: Added well-documented API method for checking plugin modifications.

This method declaration includes comprehensive XML documentation that clearly explains its purpose, the meaning of the id parameter, and when a plugin is considered modified. The documentation helps users understand the implications of plugin modifications.


373-383: Added well-documented API method for updating plugins.

This method declaration includes detailed XML documentation that clearly explains its purpose, parameters, and behavior regarding zip file handling. The documentation is thorough and helps users understand how the plugin update process works.


385-393: Added well-documented API method for installing plugins.

This method declaration includes comprehensive XML documentation that clearly explains its purpose, parameters, and behavior regarding zip file handling. The documentation is consistent with the update method documentation.


395-403: Added well-documented API method for uninstalling plugins.

This method declaration includes thorough XML documentation that clearly explains its purpose, parameters, and the optional behavior of removing plugin settings. The documentation helps users understand the implications of plugin uninstallation.

Flow.Launcher/SettingPages/ViewModels/SettingsPanePluginStoreViewModel.cs (2)

15-21: Using the new API abstraction looks good!

This refactoring improves the code's architecture by accessing the plugin manifest through the API layer (App.API.GetPluginManifest()) instead of directly accessing PluginsManifest.UserPlugins. This change promotes better separation of concerns and increases maintainability.


26-26:

❓ Verification inconclusive

API usage improvement looks good, but consider parameter usage

The change to use App.API.UpdatePluginManifestAsync() instead of the direct manifest call is a good architectural improvement.

However, according to the PR description, this method accepts a usePrimaryUrlOnly parameter, which isn't being utilized here. Consider whether you should be passing a specific value for this parameter based on your use case.


🏁 Script executed:

#!/bin/bash
# Check the signature of the UpdatePluginManifestAsync method to confirm parameter usage
rg -A 3 "UpdatePluginManifestAsync" --type cs

Length of output: 2520


Consider Explicitly Passing the usePrimaryUrlOnly Parameter

The architectural improvement is solid. However, note that the UpdatePluginManifestAsync method accepts a usePrimaryUrlOnly parameter (defaulting to false), and here it’s being invoked without an explicit value. Please verify whether relying on the default (false) is intentional or if a specific value should be provided based on the intended behavior—especially since elsewhere (e.g., in PluginsManager) the parameter is explicitly passed.

@Jack251970 Jack251970 requested a review from Copilot April 4, 2025 09:23
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 14 out of 15 changed files in this pull request and generated no comments.

Files not reviewed (1)
  • Plugins/Flow.Launcher.Plugin.PluginsManager/Flow.Launcher.Plugin.PluginsManager.csproj: Language not supported

Jack251970 and others added 2 commits April 4, 2025 17:24
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

This comment has been minimized.

Copy link

github-actions bot commented Apr 4, 2025

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

❌ Errors Count
❌ forbidden-pattern 22
⚠️ non-alpha-in-dictionary 19

See ❌ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (6)
Flow.Launcher.Plugin/UserPlugin.cs (6)

35-38: Fix reference to non-existent AllowedLanguage class in XML documentation.

The XML documentation for the Language property references a class AllowedLanguage using the <see cref="AllowedLanguage"/> tag, but this class doesn't appear to be defined in the given code or context. Either include the definition of AllowedLanguage or update the documentation to correctly describe the expected values.

/// <summary>
-/// Allow language of the plugin <see cref="AllowedLanguage"/>
+/// Language of the plugin (e.g., "csharp", "python", etc.)
/// </summary>
public string Language { get; set; }

8-9: Consider using init-only properties with record for better immutability.

Records in C# are typically used to model immutable data. Currently, all properties have public setters, making them mutable. Consider using init-only properties to enforce immutability, which aligns better with the record concept and could prevent accidental modifications after initialization.

public record UserPlugin
{
    /// <summary>
    /// Unique identifier of the plugin
    /// </summary>
-   public string ID { get; set; }
+   public string ID { get; init; }

    // Apply similar changes to other properties
}

30-33: Use System.Version for better version handling.

Using a plain string for versions doesn't provide validation or comparison capabilities. Consider using System.Version which provides built-in parsing, validation, and comparison functionality for version strings.

/// <summary>
/// Version of the plugin
/// </summary>
-public string Version { get; set; }
+public Version Version { get; set; }

This would require version strings to follow semantic versioning format but provides better type safety and comparison capabilities.


13-13: Consider non-nullability for essential properties.

Essential properties like ID should never be null. Consider using non-nullable reference types (with #nullable enable) or adding validation to ensure critical properties are not null.

/// <summary>
/// Unique identifier of the plugin
/// </summary>
-public string ID { get; set; }
+public string ID { get; set; } = string.Empty;

Alternatively, if using C# 8.0 or later:

#nullable enable
public string ID { get; set; } = null!;

1-4: Consider adding appropriate using directives for serialization.

Based on the provided code snippet from GetPluginInfoFromZip, this class is deserialized from JSON. Consider adding the appropriate System.Text.Json attributes to control serialization/deserialization behavior.

using System;
+using System.Text.Json.Serialization;

namespace Flow.Launcher.Plugin

Then you can add attributes to properties if needed:

[JsonPropertyName("id")]
public string ID { get; set; }

13-73: Add validation for required properties.

There's no validation to ensure required properties are properly set. Consider implementing a validation method or constructor that validates essential properties.

public record UserPlugin
{
    // Properties...
    
    /// <summary>
    /// Validates that all required properties are set
    /// </summary>
    /// <exception cref="InvalidOperationException">Thrown when a required property is missing</exception>
    public void Validate()
    {
        if (string.IsNullOrEmpty(ID))
            throw new InvalidOperationException("Plugin ID is required");
        
        if (string.IsNullOrEmpty(Name))
            throw new InvalidOperationException("Plugin Name is required");
            
        // Add validation for other required properties
    }
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 473b139 and 171ebe9.

📒 Files selected for processing (1)
  • Flow.Launcher.Plugin/UserPlugin.cs (1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
Flow.Launcher.Plugin/UserPlugin.cs (1)
Plugins/Flow.Launcher.Plugin.PluginsManager/Utilities.cs (1)
  • UserPlugin (62-80)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm

@Jack251970 Jack251970 added the enhancement New feature or request label Apr 4, 2025
@@ -1,5 +1,6 @@
using Flow.Launcher.Infrastructure.Http;
using Flow.Launcher.Infrastructure.Logger;
using Flow.Launcher.Plugin;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed?

Record class UserPlugin is moved to Flow.Launcher.Plugin, so we need to use this namespace here

using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Flow.Launcher.Plugin;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed?

Record class UserPlugin is moved to Flow.Launcher.Plugin, so we need to use this namespace here

/// Update the plugin manifest
/// </summary>
/// <param name="usePrimaryUrlOnly">
/// FL has multiple urls to download the plugin manifest. Set this to true to only use the primary url.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we added support for multiple manifest urls or are they just mirrors?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just mirrors

image

@@ -1,5 +1,4 @@
using Flow.Launcher.Core.ExternalPlugins;
using ICSharpCode.SharpZipLib.Zip;
using ICSharpCode.SharpZipLib.Zip;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed I believe

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use ZipInputStream and ZipEntry in function Unzip, so we need to use this namespace.

image

</ItemGroup>

<ItemGroup>
<PackageReference Include="SharpZipLib" Version="1.4.2" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is used right?

Previous version also refers to this package as transitive package from Flow.Launcher.Core project becausesquirrel.windows package use SharpZipLib package. Since this PR drops project reference of Flow.Launcher.Core project, we need to add this package reference.

@Jack251970 Jack251970 requested a review from jjw24 April 5, 2025 08:41
Copy link
Member

@jjw24 jjw24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jjw24 jjw24 merged commit 6ebb73b into Flow-Launcher:dev Apr 5, 2025
7 checks passed
@jjw24 jjw24 added this to the 1.20.0 milestone Apr 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants