Skip to content

Conversation

Jack251970
Copy link
Member

Fix possible image recursive loading

If this image is missing, _image == ImageLoader.MissingImage will always be true, which can cause recursive loading. So we should use loaded flag to indicate this

@prlabeler prlabeler bot added the bug Something isn't working label Aug 13, 2025
@Jack251970 Jack251970 requested a review from Copilot August 13, 2025 06:05
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.

Pull Request Overview

This PR fixes a potential infinite recursion issue in image loading where missing images could cause repeated loading attempts. The change replaces the missing image check with a loaded flag to prevent multiple loading operations.

  • Introduces a boolean flag to track image loading state
  • Replaces missing image comparison with loaded flag check to prevent recursive loading

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

gitstream-cm bot commented Aug 13, 2025

🥷 Code experts: no user but you matched threshold 10

Jack251970 has most 👩‍💻 activity in the files.
Jack251970, Yusyuriv have most 🧠 knowledge in the files.

See details

Flow.Launcher/ViewModel/PluginViewModel.cs

Activity based on git-commit:

Jack251970
AUG
JUL
JUN 3 additions & 3 deletions
MAY 19 additions & 25 deletions
APR 38 additions & 54 deletions
MAR 94 additions & 57 deletions

Knowledge based on git-blame:
Jack251970: 46%
Yusyuriv: 11%

✨ Comment /gs review for LinearB AI review. Learn how to automate it here.

Copy link

gitstream-cm bot commented Aug 13, 2025

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

@Jack251970 Jack251970 added this to the 2.0.0 milestone Aug 13, 2025
@Jack251970 Jack251970 enabled auto-merge August 13, 2025 06:10
Copy link
Contributor

coderabbitai bot commented Aug 13, 2025

📝 Walkthrough

Walkthrough

Introduced a one-time lazy initialization gate for asynchronous icon loading in PluginViewModel.Image using a new _imageLoaded flag, replacing the previous MissingImage-based trigger. LoadIconAsync behavior and public API remain unchanged.

Changes

Cohort / File(s) Summary
ViewModel Icon Lazy Load
Flow.Launcher/ViewModel/PluginViewModel.cs
Added _imageLoaded flag and updated Image getter to trigger LoadIconAsync() once on first access instead of checking ImageLoader.MissingImage. LoadIconAsync still sets Image and raises OnPropertyChanged. No public API changes.

Sequence Diagram(s)

sequenceDiagram
    participant UI as UI/Consumer
    participant VM as PluginViewModel
    participant Loader as LoadIconAsync

    UI->>VM: Access Image
    alt First access (_imageLoaded == false)
        VM->>VM: _imageLoaded = true
        VM->>Loader: LoadIconAsync()
        activate Loader
        Loader-->>VM: Set Image, OnPropertyChanged(Image)
        deactivate Loader
    else Subsequent access
        VM-->>UI: Return cached Image
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Suggested reviewers

  • taooceros
  • onesounds
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch recursive_loading

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 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.
    • 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.
  • 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 the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

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

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • 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: 1

🧹 Nitpick comments (1)
Flow.Launcher/ViewModel/PluginViewModel.cs (1)

40-41: Rename _imageLoaded to _imageLoadStarted for semantic clarity

The flag is set before loading completes and is used as a “started” gate, not a “loaded” indicator. Renaming reduces confusion and aligns field name with behavior.

Apply this diff:

-        private bool _imageLoaded = false;
+        private bool _imageLoadStarted = false;
@@
-                if (!_imageLoaded)
+                if (!_imageLoadStarted)
                 {
-                    _imageLoaded = true;
+                    _imageLoadStarted = true;
                     _ = LoadIconAsync();
                 }

Alternatively, if you truly want a “loaded” (completed) signal, set a separate flag at the end of LoadIconAsync and keep this one as “started.” This also gives you a place to decide whether/how to retry if the first attempt fails.

Also applies to: 46-51

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between 7b08c96 and 2aa07a2.

📒 Files selected for processing (1)
  • Flow.Launcher/ViewModel/PluginViewModel.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-01T05:46:13.251Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.

Applied to files:

  • Flow.Launcher/ViewModel/PluginViewModel.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: gitStream.cm
  • GitHub Check: gitStream.cm
  • GitHub Check: build

@Jack251970 Jack251970 merged commit cc42cd6 into dev Aug 13, 2025
16 checks passed
@Jack251970 Jack251970 deleted the recursive_loading branch August 13, 2025 06:30
TBM13 pushed a commit to TBM13/Flow.Launcher that referenced this pull request Aug 13, 2025
…ading

Fix possible image recursive loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants