-
-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WebSearch] Add copy URL to menu #3082
Conversation
This comment has been minimized.
This comment has been minimized.
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces new string resources for English and Russian language support in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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
🧹 Outside diff range and nitpick comments (3)
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xaml (1)
34-35
: LGTM with a minor naming suggestion!The new string resources are well-structured and clearly describe the copy URL functionality. However, consider using underscore for consistency in the key names:
flowlauncher_plugin_websearch_copy_url_title
flowlauncher_plugin_websearch_copy_url_subtitle
This matches the pattern used in other keys like
flowlauncher_plugin_websearch_action_keyword
.- <system:String x:Key="flowlauncher_plugin_websearch_copyurl_title">Copy URL</system:String> - <system:String x:Key="flowlauncher_plugin_websearch_copyurl_subtitle">Copy search URL to clipboard</system:String> + <system:String x:Key="flowlauncher_plugin_websearch_copy_url_title">Copy URL</system:String> + <system:String x:Key="flowlauncher_plugin_websearch_copy_url_subtitle">Copy search URL to clipboard</system:String>Plugins/Flow.Launcher.Plugin.WebSearch/Main.cs (2)
79-80
: Consider extracting URL generation to reduce duplicationThe URL generation logic is duplicated between the Action and ContextData assignments. Consider extracting this into a local variable to improve maintainability.
+ var generatedUrl = searchSource.Url.Replace("{q}", Uri.EscapeDataString(keyword)); Action = c => { - _context.API.OpenUrl(searchSource.Url.Replace("{q}", Uri.EscapeDataString(keyword))); + _context.API.OpenUrl(generatedUrl); return true; }, - ContextData = searchSource.Url.Replace("{q}", Uri.EscapeDataString(keyword)), + ContextData = generatedUrl,
143-144
: Apply the same URL generation extraction hereSimilar to the previous comment, the URL generation logic is duplicated. Apply the same refactoring pattern here.
+ var generatedUrl = searchSource.Url.Replace("{q}", Uri.EscapeDataString(o)); Action = c => { - _context.API.OpenUrl(searchSource.Url.Replace("{q}", Uri.EscapeDataString(o))); + _context.API.OpenUrl(generatedUrl); return true; }, - ContextData = searchSource.Url.Replace("{q}", Uri.EscapeDataString(o)), + ContextData = generatedUrl,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Plugins/Flow.Launcher.Plugin.WebSearch/Images/copylink.png
is excluded by!**/*.png
📒 Files selected for processing (3)
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/Languages/ru.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.WebSearch/Main.cs
(3 hunks)
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.WebSearch/Languages/ru.xaml (1)
32-33
: LGTM! Verify translation consistency.
The Russian translations appear accurate and well-formatted:
- "Скопировать URL-адрес" → "Copy URL"
- "Скопировать URL поиска в буфер обмена" → "Copy search URL to clipboard"
Let's verify the corresponding English strings exist:
✅ Verification successful
Russian translations are accurate and consistent with English strings ✓
The Russian translations perfectly match their English counterparts:
flowlauncher_plugin_websearch_copyurl_title
:- EN: "Copy URL"
- RU: "Скопировать URL-адрес"
flowlauncher_plugin_websearch_copyurl_subtitle
:- EN: "Copy search URL to clipboard"
- RU: "Скопировать URL поиска в буфер обмена"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the English translations exist in en.xaml
# Expected: Should find both string keys with their English translations
rg "flowlauncher_plugin_websearch_copyurl_title" "Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xaml"
rg "flowlauncher_plugin_websearch_copyurl_subtitle" "Plugins/Flow.Launcher.Plugin.WebSearch/Languages/en.xaml"
Length of output: 429
Plugins/Flow.Launcher.Plugin.WebSearch/Main.cs (1)
14-14
: LGTM: Clean interface implementation
The addition of IContextMenu
interface is appropriate for implementing context menu functionality.
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. If the flagged items are 🤯 false positivesIf items relate to a ...
|
🥷 Code experts: jjw24 jjw24 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: To learn more about /:\ gitStream - Visit our Docs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's hold off merging this in until we do 1.20.0 release.
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
URL ->
ContextData
-> Context Menu