Prefer https over http setting for url #4037
Conversation
|
🥷 Code experts: jjw24, Jack251970 Jack251970 has most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 WalkthroughWalkthroughAdds an "Always open with HTTPS" boolean setting and UI to the Url plugin, updates Main to implement ISettingProvider and use that preference when normalizing URLs, adds localization entries, and wraps URL opening in a try/catch. Includes ViewModel and XAML UserControl for settings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as URLSettings (UI)
participant Main as Url Plugin Main
participant Settings as Settings
participant API as Flow API
User->>UI: Toggle "Prefer https over http"
UI->>Settings: Update AlwaysOpenWithHttps (TwoWay bind)
User->>Main: Enter URL (without scheme)
Main->>Settings: Read AlwaysOpenWithHttps
alt AlwaysOpenWithHttps = true
Main->>Main: Scheme = "https"
else
Main->>Main: Scheme = "http"
end
Main->>Main: Normalize URL with selected scheme
Main->>API: OpenUrl(normalized)
alt OpenUrl throws
API--x Main: Exception
Main->>API: Show error message
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)
⏰ 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)
🔇 Additional comments (1)
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: 1
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Url/SettingsViewModel.cs (1)
1-2: Remove unused imports.Neither
System.Collections.GenericnorSystem.Linqare used in this file.Apply this diff:
-using System.Collections.Generic; -using System.Linq; - namespace Flow.Launcher.Plugin.Url;Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml.cs (1)
7-7: Consider removing the unused field.The
_viewModelfield is assigned but never read. SinceDataContextis set directly, the field is not needed.Apply this diff if you want to simplify:
public partial class URLSettings : UserControl { - private readonly SettingsViewModel _viewModel; - public URLSettings(Settings settings) { - _viewModel = new SettingsViewModel(settings); - DataContext = _viewModel; + DataContext = new SettingsViewModel(settings); InitializeComponent(); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Plugins/Flow.Launcher.Plugin.Url/Languages/de.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Url/Main.cs(4 hunks)Plugins/Flow.Launcher.Plugin.Url/Settings.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Url/SettingsViewModel.cs(1 hunks)Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
Plugins/Flow.Launcher.Plugin.Url/SettingsViewModel.cs (2)
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
Settings(3-9)Plugins/Flow.Launcher.Plugin.ProcessKiller/ViewModels/SettingsViewModel.cs (1)
SettingsViewModel(3-6)
Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml.cs (2)
Plugins/Flow.Launcher.Plugin.Url/SettingsViewModel.cs (1)
SettingsViewModel(6-9)Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
Settings(3-9)
Plugins/Flow.Launcher.Plugin.Url/Main.cs (1)
Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml.cs (2)
URLSettings(5-15)URLSettings(9-14)
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml
[warning] 28-28:
usehttps is not a recognized word. (unrecognized-spelling)
🔇 Additional comments (8)
Plugins/Flow.Launcher.Plugin.Url/Languages/de.xaml (1)
17-19: LGTM!The German localization for the HTTPS preference setting is properly added and the translation is accurate.
Plugins/Flow.Launcher.Plugin.Url/Settings.cs (1)
8-8: LGTM!The new
AlwaysOpenWithHttpsproperty is well-named with a sensible default that maintains backward compatibility.Plugins/Flow.Launcher.Plugin.Url/SettingsViewModel.cs (1)
6-9: LGTM!The ViewModel structure is clean and follows the pattern used in other plugins (e.g., ProcessKiller plugin). The primary constructor syntax is modern and concise.
Plugins/Flow.Launcher.Plugin.Url/Languages/en.xaml (1)
18-19: LGTM!The English localization for the HTTPS preference setting is properly added and clearly worded.
Plugins/Flow.Launcher.Plugin.Url/URLSettings.xaml (1)
1-31: LGTM!The settings UI is properly structured with correct two-way binding for the checkbox and dynamic resource lookup for localization. The static analysis warning about "usehttps" is a false positive—it's part of a valid resource key name.
Plugins/Flow.Launcher.Plugin.Url/Main.cs (3)
66-76: LGTM!The try-catch block properly handles URL opening failures and displays an error message to the user, improving the robustness of the plugin.
84-87: LGTM!The
GetHttpPreference()helper method is clear and concise, encapsulating the preference logic.
123-126: LGTM!The
CreateSettingPanel()implementation correctly returns a settings UI bound to the plugin's settings instance.
Closes #3773
This is still missing some translations though and it also adds a settings panel to the url plugin.
There are some other unused properties in this plugin, not sure if these can be deleted?
Also default behaviour is http but I think https should be the state of the art at this point