Support .url Internet Shortcut File Icons #4006
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds ini-parser v2.5.2 and updates ThumbnailReader to handle Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Caller as Caller
participant TR as ThumbnailReader
participant FS as FileSystem
participant INI as IniParser
participant Icon as IconLoader
Caller->>TR: GetThumbnail(path)
alt path ends with ".url"
TR->>FS: Read file contents
TR->>INI: Parse INI (InternetShortcut)
INI-->>TR: IconFile (optional)
alt IconFile present
TR->>Icon: Load icon from IconFile
Icon-->>TR: HBitmap
TR-->>Caller: Thumbnail from icon
else IconFile missing/failed
TR->>TR: Default thumbnail flow (fallback)
TR-->>Caller: Default thumbnail
end
else other file types
TR->>TR: Default thumbnail flow
TR-->>Caller: Default thumbnail
end
note right of TR: New branch parses .url INI and attempts icon-based thumbnailing
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
![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 (2)
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (2)
45-53: Prefer explicit ordinal comparison; avoid ToLowerInvariant allocation.Clearer and cheaper string compare.
- var extension = Path.GetExtension(fileName)?.ToLowerInvariant(); - if (extension is UrlExtension) + var extension = Path.GetExtension(fileName); + if (string.Equals(extension, UrlExtension, StringComparison.OrdinalIgnoreCase)) { hBitmap = GetHBitmapForUrlFile(fileName, width, height, options); } else { hBitmap = GetHBitmap(Path.GetFullPath(fileName), width, height, options); }
41-64: Alternative (simpler) approach: ask Shell for the .url’s icon directly.You may get correct custom icons without parsing by forcing IconOnly for .url; reduces dependency surface. Worth A/B testing.
- if (string.Equals(extension, UrlExtension, StringComparison.OrdinalIgnoreCase)) - { - hBitmap = GetHBitmapForUrlFile(fileName, width, height, options); - } + if (string.Equals(extension, UrlExtension, StringComparison.OrdinalIgnoreCase)) + { + // Shell should honor custom IconFile for .url when requesting icons. + hBitmap = GetHBitmap(Path.GetFullPath(fileName), width, height, ThumbnailOptions.IconOnly); + }Please verify whether this alone shows assigned icons for .url files (custom icon set via “Change Icon…”).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Core/packages.lock.json(2 hunks)Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj(1 hunks)Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs(3 hunks)Flow.Launcher.Infrastructure/packages.lock.json(1 hunks)Flow.Launcher/packages.lock.json(2 hunks)
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2024-10-08T15:52:58.573Z
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Applied to files:
Flow.Launcher.Core/packages.lock.jsonFlow.Launcher.Infrastructure/packages.lock.jsonFlow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj
📚 Learning: 2025-09-04T11:52:29.096Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.096Z
Learning: In Flow Launcher's PluginsManifest.cs, when dealing with version parsing for the MinimumAppVersion feature, maintainer jjw24 prefers to keep the solution simple rather than implementing comprehensive helper methods for SemVer parsing normalization.
Applied to files:
Flow.Launcher.Core/packages.lock.jsonFlow.Launcher/packages.lock.json
📚 Learning: 2025-02-28T07:47:24.148Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3279
File: Flow.Launcher/Helper/WallpaperPathRetrieval.cs:44-46
Timestamp: 2025-02-28T07:47:24.148Z
Learning: In Flow.Launcher's WallpaperPathRetrieval class, using a `using` statement with MemoryStream when loading images with BitmapImage does not work properly, even when using BitmapCacheOption.OnLoad. The stream needs to remain open while the bitmap is in use.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-08-13T06:12:43.382Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.382Z
Learning: In Flow Launcher's PluginViewModel.cs, the LoadIconAsync method does not require additional try-catch error handling according to maintainer Jack251970, as the existing error handling approach is considered sufficient for the image loading operations.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
🔇 Additional comments (7)
Flow.Launcher.Infrastructure/packages.lock.json (1)
32-37: Lock entry added — OK.Direct dependency captured with correct version/hash. Keep this in sync with other lockfiles after any subsequent restores.
Flow.Launcher.Core/packages.lock.json (2)
86-90: Transitive addition acknowledged.Core now records ini-parser as transitive. No action needed beyond ensuring CI restores in locked mode.
271-273: Infrastructure project deps updated — OK.Dependency fan‑out correctly includes ini-parser.
Flow.Launcher/packages.lock.json (2)
207-211: Root lock transitive entry — OK.Matches infra/core locks. Keep hashes aligned after any restore.
871-873: Project dependency list updated — OK.ini-parser included with other infra deps; nothing else to do.
Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj (1)
63-63: Add ini-parser to third‑party notices and regenerate package locks
- Found in: Plugins/Flow.Launcher.Plugin.Program/Flow.Launcher.Plugin.Program.csproj and Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj; packages.lock.json in Flow.Launcher, Flow.Launcher.Core, Flow.Launcher.Infrastructure show resolved: 2.5.2.
- Action: Add ini-parser to third‑party notices per repo policy; run
dotnet restore --locked-modewith the .NET SDK (CI or locally) to validate/regenerate packages.lock.json and commit any updated lockfiles.- Verification:
dotnet restore --locked-modecould not be executed in the verification sandbox (dotnet: command not found).Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (1)
125-148: .url IconFile parsing must handle IconIndex, file.dll, env vars, quotes, and relative pathsFile: Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (lines 125–148). Repo search for ExtractIconEx/SHDefExtractIcon/HICON/GetIconInfo returned no matches — no reusable icon-extraction helper found.
Minimum fix: normalize and resolve IconFile (trim quotes, ExpandEnvironmentVariables, resolve relative to the .url directory, split off optional ",idx" even if index is ignored). Apply the diff below.
- try - { - var parser = new FileIniDataParser(); - var data = parser.ReadFile(fileName); - var urlSection = data["InternetShortcut"]; - - var iconPath = urlSection?["IconFile"]; - if (string.IsNullOrEmpty(iconPath)) - { - throw new FileNotFoundException(); - } - hBitmap = GetHBitmap(Path.GetFullPath(iconPath), width, height, options); - } + try + { + var parser = new FileIniDataParser(); + var data = parser.ReadFile(fileName); + var urlSection = data["InternetShortcut"]; + + var iconPathRaw = urlSection?["IconFile"]?.Trim().Trim('"'); + if (string.IsNullOrWhiteSpace(iconPathRaw)) + throw new FileNotFoundException(); + + // Expand %ENV% and resolve relative to .url location + var expanded = Environment.ExpandEnvironmentVariables(iconPathRaw); + var baseDir = Path.GetDirectoryName(Path.GetFullPath(fileName))!; + var resolved = Path.IsPathRooted(expanded) ? expanded : Path.GetFullPath(Path.Combine(baseDir, expanded)); + + // Handle "file,idx" pattern (idx may be negative). We ignore idx for now (see TODO below). + var (iconFileOnly, _) = SplitIconLocation(resolved); + + hBitmap = GetHBitmap(Path.GetFullPath(iconFileOnly), width, height, options); + } catch { hBitmap = GetHBitmap(Path.GetFullPath(fileName), width, height, options); }Add helper (outside the shown range):
private static (string file, int? index) SplitIconLocation(string iconLocation) { // e.g., "C:\\Path\\imageres.dll,15" or "C:\\Path\\file.ico" // Find last comma not within quotes var s = iconLocation.Trim(); var lastComma = s.LastIndexOf(','); if (lastComma > 0 && lastComma < s.Length - 1 && int.TryParse(s[(lastComma + 1)..].Trim(), out var idx)) return (s[..lastComma].Trim().Trim('"'), idx); return (s.Trim().Trim('"'), null); }Follow‑ups (required): properly honor IconIndex / file,idx by extracting that specific icon (SHDefExtractIcon/ExtractIconEx → HICON → convert to HBITMAP); add unit tests for absolute .ico, file.dll,idx, env-var paths, quoted paths, and relative paths.
|
@coderabbitai generate docstrings |
✅ Actions performedDocstrings generation started. |
Docstrings generation was requested by @sstream17. * #4006 (comment) The following files were modified: * `Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs`
|
Note Generated docstrings for this pull request at #4007 |
There was a problem hiding this comment.
Pull Request Overview
Adds support for displaying icons from .url Internet shortcut files in Flow Launcher by parsing the shortcut's INI file to extract the IconFile path.
- Adds ini-parser NuGet package dependency for parsing .url files
- Implements special handling for .url files in the thumbnail reader to extract and display their associated icons
- Maintains fallback behavior to default system icons when IconFile is not specified or accessible
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Flow.Launcher.Infrastructure/ThumbnailReader.cs | Adds logic to parse .url files and extract icon paths from IconFile entries |
| Flow.Launcher.Infrastructure/Flow.Launcher.Infrastructure.csproj | Adds ini-parser package reference |
| Flow.Launcher/packages.lock.json | Updates lock file with ini-parser transitive dependency |
| Flow.Launcher.Infrastructure/packages.lock.json | Updates lock file with direct ini-parser dependency |
| Flow.Launcher.Core/packages.lock.json | Updates lock file with ini-parser transitive dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
|
@sstream17 It does not work for me. 
|
Sorry about that, I updated the instructions to be a little clearer. As for the icon not appearing for the UWP Apps shortcut, does that shortcut have an icon? This Git FAQs shortcut has an icon: But this Node.js shortcut is missing an icon: |
Sorry, I didn't realize this would spin off a new PR. |
|
Tip For best results, initiate chat on the files or code changes. @sstream17 No need to apologize! It's completely understandable that you weren't expecting a separate PR to be created. The docstring generation creates a separate PR (#4007) to keep the generated documentation changes isolated from your main feature work. This approach allows you to:
You can review PR #4007 and if the docstrings look good, you can merge that PR independently. If you'd prefer to have the docstrings included directly in this PR instead, let me know and I can help guide you through alternative approaches for future requests. Your main PR (#4006) for the .url file icon support looks great and can proceed independently! |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (3)
129-134: INI parsing path robustness (minor).Some .url files may differ in casing (“InternetShortcut”, “Internetshortcut”). Consider tolerant access or a null-check with a clearer message if the section is missing.
143-154: Optional: narrow the catch or log the root cause.The blanket catch is fine for a visual fallback, but it obscures why the url-path path failed. If you have central logging, log the original exception before falling back.
If logging is available, add a trace here; otherwise ignore.
135-142: Exact icon selection via IconIndex (nice-to-have).For full fidelity with shortcuts that specify a non-default icon in an EXE/DLL, use IconIndex (or parsed “,index”) to extract that specific icon (e.g., via ExtractIconEx) instead of relying on the default icon of the file.
I can provide a small helper using ExtractIconEx to get an HICON for (path, index) and render it to an HBITMAP if you want to include this now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs(3 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-02-28T07:47:24.148Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3279
File: Flow.Launcher/Helper/WallpaperPathRetrieval.cs:44-46
Timestamp: 2025-02-28T07:47:24.148Z
Learning: In Flow.Launcher's WallpaperPathRetrieval class, using a `using` statement with MemoryStream when loading images with BitmapImage does not work properly, even when using BitmapCacheOption.OnLoad. The stream needs to remain open while the bitmap is in use.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-08-13T06:12:43.382Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3897
File: Flow.Launcher/ViewModel/PluginViewModel.cs:46-51
Timestamp: 2025-08-13T06:12:43.382Z
Learning: In Flow Launcher's PluginViewModel.cs, the LoadIconAsync method does not require additional try-catch error handling according to maintainer Jack251970, as the existing error handling approach is considered sufficient for the image loading operations.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-06-08T14:12:12.842Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:318-318
Timestamp: 2025-06-08T14:12:12.842Z
Learning: In Flow.Launcher, the App.NotifyIcon static property is initialized in the App class before MainWindow creation, so null checks are not needed when accessing App.NotifyIcon in MainWindow lifecycle methods.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
📚 Learning: 2025-06-08T14:12:21.348Z
Learnt from: Jack251970
PR: Flow-Launcher/Flow.Launcher#3672
File: Flow.Launcher/MainWindow.xaml.cs:244-247
Timestamp: 2025-06-08T14:12:21.348Z
Learning: In Flow.Launcher, App.NotifyIcon is created before MainWindow creation, so null checks for App.NotifyIcon are not necessary when accessing it from MainWindow code.
Applied to files:
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs
🔇 Additional comments (1)
Flow.Launcher.Infrastructure/Image/ThumbnailReader.cs (1)
41-53: Solid .url detection and delegation.The early branch to a dedicated handler keeps the main path unchanged and limits risk. Looks good.
This reverts commit 49d5cd3.
Support .url Internet Shortcut File Icons
Support .url Internet Shortcut File Icons
This change adds support for icons in internet shortcuts with .url extension. Resolve #2850
Limit: It cannot resolve icons that from website.
Before this change
.url files don't show their icons in Flow Launcher.
While investigating how to fix this, I discovered #1476. That PR added support for showing .url files in the Program plugin.
I found if the .url file is in Start Menu/Programs and the correct settings in the Program plugin are enabled, .url files can show an icon.
This example shows the difference between the Program plugin with a working icon and the same .url file.
After this change
Icons will appear for all .url files, regardless of the file location or if the HTTP/HTTPS File Type setting is on:
Test
"C:\ProgramData\Microsoft\Windows\Start Menu\Programs\Git\Git FAQs (Frequently Asked Questions).url".