Skip to content

Conversation

Jack251970
Copy link
Member

@Jack251970 Jack251970 commented Sep 4, 2025

Support Custom Browser Path & Open in window / tab & In private for URL Plugin

When open one url, we can set custom browser path, choose whether to open in new window or tab, and choose whether to open in private mode.

Test

image
  • Setting a custom browser path can work.
  • Opening in new window can work.
  • Opening in private can work.

@Jack251970 Jack251970 added this to the Future milestone Sep 4, 2025
@github-actions github-actions bot modified the milestones: Future, 2.0.0 Sep 4, 2025
@Jack251970 Jack251970 requested a review from Copilot September 4, 2025 14:09
Copy link

gitstream-cm bot commented Sep 4, 2025

🥷 Code experts: jjw24

Jack251970 has most 👩‍💻 activity in the files.
jjw24 has most 🧠 knowledge in the files.

See details

Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml

Activity based on git-commit:

Jack251970
SEP
AUG
JUL
JUN
MAY
APR

Knowledge based on git-blame:
jjw24: 33%

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

Activity based on git-commit:

Jack251970
SEP
AUG
JUL 1 additions & 1 deletions
JUN
MAY
APR

Knowledge based on git-blame:
jjw24: 4%

Plugins/Flow.Launcher.Plugin.Url/Settings.cs

Activity based on git-commit:

Jack251970
SEP
AUG
JUL
JUN
MAY
APR

Knowledge based on git-blame:
jjw24: 22%

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

Copy link

gitstream-cm bot commented Sep 4, 2025

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

@coderabbitai coderabbitai bot added the enhancement New feature or request label Sep 4, 2025
Copy link
Contributor

coderabbitai bot commented Sep 4, 2025

Warning

Rate limit exceeded

@Jack251970 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a8c64b and ec41ec2.

📒 Files selected for processing (2)
  • Plugins/Flow.Launcher.Plugin.Url/Main.cs (5 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (1 hunks)

Note

Other AI code review bot(s) detected

CodeRabbit 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.

📝 Walkthrough

Walkthrough

Adds a bindable Settings model and WPF settings UI for the Url plugin, new XAML localization keys and converters, and updates Main to implement ISettingProvider, expose static Context/Settings, and change URL detection/opening to honor custom browser, tab/window, and private-mode settings.

Changes

Cohort / File(s) Summary
Localization
Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml
Reworked ResourceDictionary header formatting; removed several old keys and added new keys for custom browser toggle, browser path, new tab/window labels, and private mode.
Plugin core / settings provider
Plugins/Flow.Launcher.Plugin.Url/Main.cs
Main now implements ISettingProvider; introduces public static PluginInitContext Context and public static Settings Settings; adds CreateSettingPanel(); replaces instance regex with static UrlRegex; URL handling now respects Settings (UseCustomBrowser, BrowserPath, OpenInNewBrowserWindow, OpenInPrivateMode) and routes opening through SearchWeb or Context.API.
Settings model
Plugins/Flow.Launcher.Plugin.Url/Settings.cs
Settings now derives from BaseModel; BrowserPath and OpenInNewBrowserWindow use backing fields + OnPropertyChanged; adds UseCustomBrowser, OpenInPrivateMode, and PrivateModeArgument.
Settings UI (XAML + code-behind)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs
New SettingsControl : UserControl bound to Main.Settings; toggles custom browser panel; shows browser path (read-only) with Browse button, radio buttons for tab/window, and private-mode inputs; SelectBrowserPath opens OpenFileDialog and sets Settings.BrowserPath.
Converters
Plugins/Flow.Launcher.Plugin.Url/Converters/BoolToVisibilityConverter.cs
Plugins/Flow.Launcher.Plugin.Url/Converters/InverseBoolConverter.cs
Added BoolToVisibilityConverter (bool → Visibility) and InverseBoolConverter (bool negation) used by the settings XAML.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant P as Url Plugin (Main)
  participant S as Settings (Main.Settings)
  participant SW as SearchWeb / Context.API
  participant B as Browser

  U->>P: Enter query
  P->>P: IsURL? (UrlRegex)
  alt URL detected
    P->>S: Read UseCustomBrowser, BrowserPath, OpenInNewBrowserWindow, OpenInPrivateMode
    alt UseCustomBrowser == true
      P->>SW: OpenInBrowserTab/Window(url, BrowserPath, private?)
    else
      P->>SW: OpenInBrowserTab/Window(url, default, private?) or Context.API.OpenWebUrl
    end
    SW->>B: Launch URL (tab/window, private?)
  else Not URL
    P->>P: Continue normal search handling
  end
  note right of P: Translations & errors via Context.API
Loading
sequenceDiagram
  autonumber
  participant U as User
  participant UI as SettingsControl
  participant S as Settings
  participant D as OpenFileDialog

  U->>UI: Open settings panel
  UI->>S: Bind properties (UseCustomBrowser, BrowserPath, OpenInNewBrowserWindow, OpenInPrivateMode)
  U->>UI: Click "Browse..."
  UI->>D: ShowDialog(filter from Main.Context.API.GetTranslation)
  alt File selected
    D-->>UI: FileName
    UI->>S: Set BrowserPath (OnPropertyChanged)
  else Cancel
    D-->>UI: no selection
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • onesounds
  • taooceros

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly describes the primary enhancement—adding support for a custom browser path and options to open URLs in a window vs. tab and in private mode—and directly matches the changes in Settings, SettingsControl UI, resource updates, and Main.cs URL-opening logic, making it clear and relevant for teammates scanning history.
Description Check ✅ Passed The pull request description clearly describes the feature, includes test notes and a screenshot, and aligns with the changeset that adds settings, UI controls, converters, and updated URL-opening behavior, so it is related and on-topic.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

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

Comment @coderabbitai help to get the list of available commands and usage tips.

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: 5

🧹 Nitpick comments (7)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (2)

1-3: Add missing import for path helpers if we prefill dialog from current value.
If you adopt the improvements below (prefill InitialDirectory/FileName), you’ll need System.IO.

 using System.Windows;
 using System.Windows.Controls;
+using System.IO;

15-26: Harden file picker and improve UX (explicit checks, default ext, prefill, localized title).
Explicitly set CheckFileExists/CheckPathExists, DefaultExt, Title, and prefill from existing BrowserPath to reduce user error.

     private void SelectBrowserPath(object sender, RoutedEventArgs e)
     {
-        var dlg = new Microsoft.Win32.OpenFileDialog
-        {
-            Filter = Main.Context.API.GetTranslation("flowlauncher_plugin_url_plugin_filter")
-        };
+        var dlg = new Microsoft.Win32.OpenFileDialog
+        {
+            Filter = Main.Context.API.GetTranslation("flowlauncher_plugin_url_plugin_filter"),
+            CheckFileExists = true,
+            CheckPathExists = true,
+            DefaultExt = "exe",
+            Title = Main.Context.API.GetTranslation("flowlauncher_plugin_url_plugin_choose")
+        };
+
+        if (!string.IsNullOrWhiteSpace(Settings.BrowserPath))
+        {
+            var dir = Path.GetDirectoryName(Settings.BrowserPath);
+            if (!string.IsNullOrEmpty(dir) && Directory.Exists(dir))
+            {
+                dlg.InitialDirectory = dir;
+                dlg.FileName = Path.GetFileName(Settings.BrowserPath);
+            }
+        }
 
         if (dlg.ShowDialog() == true && !string.IsNullOrEmpty(dlg.FileName))
         {
             Settings.BrowserPath = dlg.FileName;
+            // Optional: auto-enable custom browser after choosing a path.
+            // Settings.UseCustomBrowser = true;
         }
     }
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)

7-19: Minor: normalize BrowserPath on set.
Trimming prevents accidental whitespace-only values.

-            set
+            set
             {
-                if (_browserPath != value)
+                var v = value?.Trim() ?? string.Empty;
+                if (_browserPath != v)
                 {
-                    _browserPath = value;
+                    _browserPath = v;
                     OnPropertyChanged();
                 }
             }
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml (2)

42-65: Disable path pickers when custom browser is off.
Prevents selecting a path that won’t be used and clarifies intent.

-        <Grid
+        <Grid
             Grid.Row="1"
             Grid.Column="1"
-            Margin="{StaticResource SettingPanelItemLeftTopBottomMargin}">
+            Margin="{StaticResource SettingPanelItemLeftTopBottomMargin}"
+            IsEnabled="{Binding Settings.UseCustomBrowser}">

51-57: Bind TextBox tooltip to full path for visibility.
Long paths get truncated; tooltip helps without UI clutter.

             <TextBox
                 Grid.Column="0"
                 HorizontalAlignment="Stretch"
                 VerticalAlignment="Center"
                 IsReadOnly="True"
-                Text="{Binding Settings.BrowserPath, Mode=OneWay}" />
+                Text="{Binding Settings.BrowserPath, Mode=OneWay}"
+                ToolTip="{Binding Settings.BrowserPath}" />
Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml (1)

15-18: Polish strings: capitalize “URL” and clarify private mode.

Minor UX/l10n tweak for consistency with existing strings and common terminology.

-    <system:String x:Key="flowlauncher_plugin_use_custom_browser">Use custom browser</system:String>
-    <system:String x:Key="flowlauncher_plugin_browser_path">Browser path:</system:String>
-    <system:String x:Key="flowlauncher_plugin_url_open_in_new_window">Open url in new window</system:String>
-    <system:String x:Key="flowlauncher_plugin_url_open_in_private">Open url in private</system:String>
+    <system:String x:Key="flowlauncher_plugin_use_custom_browser">Use custom browser</system:String>
+    <system:String x:Key="flowlauncher_plugin_browser_path">Browser path:</system:String>
+    <system:String x:Key="flowlauncher_plugin_url_open_in_new_window">Open URL in a new window</system:String>
+    <system:String x:Key="flowlauncher_plugin_url_open_in_private">Open URL in private mode</system:String>
Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)

44-45: Minor regex/normalization tweaks.

Make the regex static and use invariant lowercasing; IsMatch is sufficient given anchors.

-        private readonly Regex UrlRegex = new(UrlPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase);
+        private static readonly Regex UrlRegex = new(UrlPattern, RegexOptions.Compiled | RegexOptions.IgnoreCase);
@@
-            raw = raw.ToLower();
+            raw = raw.ToLowerInvariant();
@@
-            if (UrlRegex.Match(raw).Value == raw) return true;
+            if (UrlRegex.IsMatch(raw)) return true;

Also applies to: 97-101

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 738a27e and 33a9654.

📒 Files selected for processing (5)
  • Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Main.cs (3 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs
📚 Learning: 2025-04-23T15:14:49.986Z
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml
📚 Learning: 2025-09-04T11:52:29.074Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.074Z
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:

  • Plugins/Flow.Launcher.Plugin.Url/Main.cs
📚 Learning: 2025-01-18T10:10:18.414Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/Main.cs
🧬 Code graph analysis (2)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (2)
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
  • Settings (3-24)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)
  • Main (9-132)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (3)
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
  • Settings (3-24)
Flow.Launcher.Plugin/SharedCommands/SearchWeb.cs (3)
  • SearchWeb (13-128)
  • OpenInBrowserWindow (41-84)
  • OpenInBrowserTab (89-127)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (2)
  • SettingsControl (6-27)
  • SettingsControl (10-13)
⏰ 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). (2)
  • GitHub Check: build
  • GitHub Check: gitStream.cm
🔇 Additional comments (5)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (1)

8-13: LGTM: Settings property and ctor are straightforward.
No issues spotted.

Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)

21-24: Confirm default behavior change won’t surprise existing users.
Defaulting OpenInNewBrowserWindow to false may change prior behavior. Consider migrating old settings or preserving previous default.

Would you like a small one-time migration that maps any legacy “open-in-new-window/tab” settings to the new boolean?

Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml (2)

66-75: LGTM: new-window toggle binding.
Matches Main.cs logic that switches between OpenInBrowserWindow vs OpenInBrowserTab.


76-85: LGTM: private-mode toggle binding.
Assuming SearchWeb maps flags per-browser correctly.

Please sanity-check private mode with Chrome, Edge, and Firefox using a custom path (incognito/inprivate/-private-window).

Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)

55-61: Ignore C# 12 collection expression warning—project targets net9.0 (defaults to C# 12+), so […] syntax is fully supported.

Likely an incorrect or invalid review comment.

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 adds customizable browser options to the URL plugin, allowing users to specify a custom browser executable, choose between opening URLs in new windows or tabs, and enable private mode browsing.

  • Adds settings UI with checkboxes for custom browser usage, new window mode, and private browsing
  • Implements custom browser path selection with file dialog
  • Replaces default URL opening logic with configurable browser launching using SharedCommands

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
SettingsControl.xaml Creates settings UI with browser path selection and browsing option checkboxes
SettingsControl.xaml.cs Implements file dialog for custom browser path selection
Settings.cs Adds browser configuration properties with property change notification
Main.cs Updates URL opening logic to use custom browser settings and SharedCommands
Languages/en.xaml Adds localized strings for new settings options

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

@taooceros
Copy link
Member

Actually I remember this is implemented as the default browser option here?
image

@Jack251970
Copy link
Member Author

Actually I remember this is implemented as the default browser option here?
image

Yes, but this needs to use api function OpenUrl. Here I use another api in ShareCommands so we can set custom browser only for URL plugin

@Jack251970 Jack251970 modified the milestones: 2.0.0, Future Sep 5, 2025
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

♻️ Duplicate comments (2)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (2)

71-80: Private mode may no-op without an argument.

SearchWeb only adds private switches when privateArg is non-empty; if users tick “Private mode” but leave argument blank, no incognito flag is passed.

Two options:

  • Prefer central fix: map defaults inside SearchWeb based on executable name (Edge/Chrome/Firefox etc.).
  • Plugin-side fallback: infer when Settings.OpenInPrivateMode is true and Settings.PrivateModeArgument is empty.

Sample plugin-side fallback:

-                                        SearchWeb.OpenInBrowserWindow(raw, Settings.BrowserPath, Settings.OpenInPrivateMode, Settings.PrivateModeArgument);
+                                        var priv = Settings.PrivateModeArgument;
+                                        if (Settings.OpenInPrivateMode && string.IsNullOrEmpty(priv))
+                                            priv = InferPrivateArg(Settings.BrowserPath);
+                                        SearchWeb.OpenInBrowserWindow(raw, Settings.BrowserPath, Settings.OpenInPrivateMode, priv);
@@
-                                        SearchWeb.OpenInBrowserTab(raw, Settings.BrowserPath, Settings.OpenInPrivateMode, Settings.PrivateModeArgument);
+                                        var priv = Settings.PrivateModeArgument;
+                                        if (Settings.OpenInPrivateMode && string.IsNullOrEmpty(priv))
+                                            priv = InferPrivateArg(Settings.BrowserPath);
+                                        SearchWeb.OpenInBrowserTab(raw, Settings.BrowserPath, Settings.OpenInPrivateMode, priv);

And add:

private static string InferPrivateArg(string browserPath)
{
    var exe = System.IO.Path.GetFileName(browserPath)?.ToLowerInvariant();
    return exe switch
    {
        "chrome.exe" or "brave.exe" or "vivaldi.exe" => "--incognito",
        "msedge.exe" or "microsoftedge.exe" => "-inprivate",
        "firefox.exe" or "waterfox.exe" => "-private-window",
        "opera.exe" or "opera_beta.exe" or "opera_developer.exe" => "--private",
        "iexplore.exe" => "-private",
        _ => string.Empty
    };
}

Please confirm which approach you prefer; I can supply a targeted patch accordingly.


65-68: Scheme detection bug: “ftp://…” becomes “http://ftp://…”.

Use a proper absolute-URI check instead of StartsWith("http").

-                            if (!raw.StartsWith("http", StringComparison.OrdinalIgnoreCase))
-                            {
-                                raw = "http://" + raw;
-                            }
+                            if (!Uri.TryCreate(raw, UriKind.Absolute, out _))
+                            {
+                                raw = "http://" + raw;
+                            }
🧹 Nitpick comments (7)
Plugins/Flow.Launcher.Plugin.Url/Converters/BoolToVisibilityConverter.cs (1)

11-22: Make converter tolerant to null/non-bool and use NotSupportedException for ConvertBack.

Prevents brittle bindings and aligns with common WPF patterns.

-    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
-    {
-        if (value is not bool)
-            throw new ArgumentException("value should be boolean", nameof(value));
-
-        return (bool)value ? Visibility.Visible : Visibility.Collapsed;
-    }
+    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+    {
+        if (value is bool b)
+            return b ? Visibility.Visible : Visibility.Collapsed;
+        if (value is null)
+            return Visibility.Collapsed;
+        return Binding.DoNothing;
+    }
@@
-    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
-    {
-        throw new InvalidOperationException();
-    }
+    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
+    {
+        throw new NotSupportedException("BoolToVisibilityConverter is one-way.");
+    }
Plugins/Flow.Launcher.Plugin.Url/Converters/InverseBoolConverter.cs (1)

10-24: Avoid throwing on unexpected input; return Binding.DoNothing.

Safer for two-way bindings where nulls can occur during initialization.

-    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
-    {
-        if (value is not bool)
-            throw new ArgumentException("value should be boolean", nameof(value));
-
-        return !(bool)value;
-    }
+    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
+    {
+        if (value is bool b) return !b;
+        if (value is null) return Binding.DoNothing;
+        return Binding.DoNothing;
+    }
@@
-    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
-    {
-        if (value is not bool)
-            throw new ArgumentException("value should be boolean", nameof(value));
-
-        return !(bool)value;
-    }
+    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
+    {
+        if (value is bool b) return !b;
+        if (value is null) return Binding.DoNothing;
+        return Binding.DoNothing;
+    }
Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml (1)

15-21: Polish new strings for clarity (en-US).

Minor copy edits to improve readability.

-    <system:String x:Key="flowlauncher_plugin_url_use_custom_browser">Use custom web browser instead of Flow default web browser</system:String>
-    <system:String x:Key="flowlauncher_plugin_url_browser_path">Browser path</system:String>
+    <system:String x:Key="flowlauncher_plugin_url_use_custom_browser">Use a custom browser instead of Flow's default browser</system:String>
+    <system:String x:Key="flowlauncher_plugin_url_browser_path">Browser executable path</system:String>

Would you like me to add a tooltip string for the private-mode argument (e.g., examples per browser)?

Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml (3)

62-75: Improve long-path UX (trim + tooltip).

Prevents path overflow and lets users see the full value.

-                <TextBox
+                <TextBox
                     Grid.Column="0"
                     HorizontalAlignment="Stretch"
                     VerticalAlignment="Center"
                     IsReadOnly="True"
-                    Text="{Binding Settings.BrowserPath, Mode=OneWay}" />
+                    TextTrimming="CharacterEllipsis"
+                    ToolTip="{Binding Settings.BrowserPath}"
+                    Text="{Binding Settings.BrowserPath, Mode=OneWay}" />

82-84: Explicitly group the two RadioButtons.

Prevents accidental grouping with other radios in future edits.

-                <RadioButton Content="{DynamicResource flowlauncher_plugin_url_new_tab}" IsChecked="{Binding Settings.OpenInNewBrowserWindow, Converter={StaticResource InverseBoolConverter}, Mode=TwoWay}" />
-                <RadioButton Content="{DynamicResource flowlauncher_plugin_url_new_window}" IsChecked="{Binding Settings.OpenInNewBrowserWindow, Mode=TwoWay}" />
+                <RadioButton GroupName="OpenMode" Content="{DynamicResource flowlauncher_plugin_url_new_tab}" IsChecked="{Binding Settings.OpenInNewBrowserWindow, Converter={StaticResource InverseBoolConverter}, Mode=TwoWay}" />
+                <RadioButton GroupName="OpenMode" Content="{DynamicResource flowlauncher_plugin_url_new_window}" IsChecked="{Binding Settings.OpenInNewBrowserWindow, Mode=TwoWay}" />

102-114: Disable argument field unless Private mode is enabled and add guidance.

Clearer UX and avoids accidental stale args.

-                <TextBox
+                <TextBox
                     Grid.Column="0"
                     HorizontalAlignment="Stretch"
                     VerticalAlignment="Center"
-                    Text="{Binding Settings.PrivateModeArgument, Mode=TwoWay}" />
+                    IsEnabled="{Binding Settings.OpenInPrivateMode}"
+                    ToolTip="E.g., --incognito (Chrome/Brave), -inprivate (Edge), -private-window (Firefox)"
+                    Text="{Binding Settings.PrivateModeArgument, Mode=TwoWay, UpdateSourceTrigger=PropertyChanged}" />

For accessibility, consider associating the CheckBox with the “Private mode” label using AutomationProperties.LabeledBy.

Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)

101-115: Avoid culture-sensitive ToLower and simplify regex check.

Prevents unexpected casing issues with non-ASCII. Use case-insensitive ops already configured on the regex.

-            raw = raw.ToLower();
-
-            if (UrlRegex.Match(raw).Value == raw) return true;
+            if (UrlRegex.IsMatch(raw)) return true;

For the localhost checks, use OrdinalIgnoreCase:

-            if (raw == "localhost" || raw.StartsWith("localhost:") ||
-                raw == "http://localhost" || raw.StartsWith("http://localhost:") ||
-                raw == "https://localhost" || raw.StartsWith("https://localhost:")
+            if (raw.Equals("localhost", StringComparison.OrdinalIgnoreCase) || raw.StartsWith("localhost:", StringComparison.OrdinalIgnoreCase) ||
+                raw.StartsWith("http://localhost", StringComparison.OrdinalIgnoreCase) ||
+                raw.StartsWith("https://localhost", StringComparison.OrdinalIgnoreCase)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 33a9654 and 3f6bebd.

📒 Files selected for processing (6)
  • Plugins/Flow.Launcher.Plugin.Url/Converters/BoolToVisibilityConverter.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Converters/InverseBoolConverter.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Main.cs (3 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1 hunks)
  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • Plugins/Flow.Launcher.Plugin.Url/Settings.cs
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-04-23T15:14:49.986Z
Learnt from: onesounds
PR: Flow-Launcher/Flow.Launcher#0
File: :0-0
Timestamp: 2025-04-23T15:14:49.986Z
Learning: In WPF applications like Flow.Launcher, font styling should be applied using implicit styles instead of setting the FontFamily property on individual controls. Define implicit styles in a ResourceDictionary using <Style TargetType="{x:Type Button}"> format and merge it into App.xaml, which automatically applies the font to all instances of the control type while still allowing explicit overrides where needed.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml
📚 Learning: 2024-11-03T07:40:11.014Z
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in `JsonRPCPluginSettings.cs`), path validation is enabled by default in `OpenFileDialog` and `FolderBrowserDialog`, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml
📚 Learning: 2025-09-04T11:52:29.074Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#3932
File: Flow.Launcher.Core/ExternalPlugins/PluginsManifest.cs:48-55
Timestamp: 2025-09-04T11:52:29.074Z
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:

  • Plugins/Flow.Launcher.Plugin.Url/Main.cs
📚 Learning: 2025-01-18T10:10:18.414Z
Learnt from: jjw24
PR: Flow-Launcher/Flow.Launcher#2448
File: Plugins/Flow.Launcher.Plugin.Sys/ThemeSelector.cs:16-20
Timestamp: 2025-01-18T10:10:18.414Z
Learning: In Flow Launcher's plugin system, the PluginInitContext parameter passed to plugin constructors is guaranteed to be non-null by the plugin initialization system, making null checks unnecessary.

Applied to files:

  • Plugins/Flow.Launcher.Plugin.Url/Main.cs
🧬 Code graph analysis (1)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (4)
Plugins/Flow.Launcher.Plugin.BrowserBookmark/Main.cs (2)
  • Main (16-275)
  • Control (223-226)
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
  • Settings (3-50)
Flow.Launcher.Plugin/SharedCommands/SearchWeb.cs (3)
  • SearchWeb (13-128)
  • OpenInBrowserWindow (41-84)
  • OpenInBrowserTab (89-127)
Plugins/Flow.Launcher.Plugin.Url/SettingsControl.xaml.cs (2)
  • SettingsControl (6-27)
  • SettingsControl (10-13)
⏰ 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). (1)
  • GitHub Check: build
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)

118-137: Static Context/Settings pattern: OK, but confirm reload lifecycle.

If the plugin supports reloads, ensure static fields are reset appropriately on re-init to avoid stale references.

{
var dlg = new Microsoft.Win32.OpenFileDialog
{
Filter = Main.Context.API.GetTranslation("flowlauncher_plugin_url_plugin_filter")
Copy link
Member

@jjw24 jjw24 Sep 18, 2025

Choose a reason for hiding this comment

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

Don't think it's good to use a translatable string as filter, that filter shouldn't even need translation 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.

Don't think it's good to use a translatable string as filter, that filter shouldn't even need translation right?

I do not think so. In Application(*.exe)|*.exe|All files|*.*, I think Application & All files should be translated

Copy link
Member Author

Choose a reason for hiding this comment

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

Take Notepad filter for example, those filters are translated:

image

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.

Everything else LGTM other than the use of translation string as filter.

@Jack251970 Jack251970 modified the milestones: Future, 2.1.0 Sep 18, 2025
@Jack251970 Jack251970 requested a review from jjw24 September 19, 2025 08:36
Copy link
Contributor

@VictoriousRaptor VictoriousRaptor Sep 22, 2025

Choose a reason for hiding this comment

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

IIRC we've got a same converter. Do we need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIRC we've got a same converter. Do we need this?

This is used in url project. We must need to keep it.

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.

4 participants