-
-
Notifications
You must be signed in to change notification settings - Fork 386
Remove the last \\ or / in path if it exists because it can cause path type checking issues #3778
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
Conversation
…h type checking issues
🥷 Code experts: no user but you matched threshold 10 Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
1 similar comment
🥷 Code experts: no user but you matched threshold 10 Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
1 similar comment
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 Walkthrough""" WalkthroughThe code refactors the path selection logic in the Quick Access Link settings for the Explorer plugin. It introduces a local variable to store the selected path, validates it, trims trailing directory separators using Changes
Assessment against linked issues
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (14)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs
(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:34:24.926Z
Learning: In Windows Forms dialogs, Windows handles invalid paths and prevents the user from clicking "Ok" if the path is incorrect, so additional path validation is unnecessary.
Learnt from: NoPlagiarism
PR: Flow-Launcher/Flow.Launcher#3104
File: Flow.Launcher.Plugin/Result.cs:73-74
Timestamp: 2024-12-02T07:37:46.838Z
Learning: In 'Result.cs', the user prefers not to add additional data URI validation in the 'IcoPath' property setter.
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs (5)
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.
Learnt from: Yusyuriv
PR: Flow-Launcher/Flow.Launcher#3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:34:24.926Z
Learning: In Windows Forms dialogs, Windows handles invalid paths and prevents the user from clicking "Ok" if the path is incorrect, so additional path validation is unnecessary.
Learnt from: Koisu-unavailable
PR: Flow-Launcher/Flow.Launcher#3770
File: Flow.Launcher/ViewModel/MainViewModel.cs:0-0
Timestamp: 2025-06-24T19:06:48.344Z
Learning: In Flow.Launcher's Explorer plugin results, the SubTitle property always contains the directory containing the file. For file results, Title contains the filename and SubTitle contains the parent directory. For directory results, SubTitle contains the directory path itself.
Learnt from: NoPlagiarism
PR: Flow-Launcher/Flow.Launcher#3104
File: Flow.Launcher.Plugin/Result.cs:73-74
Timestamp: 2024-12-02T07:37:46.838Z
Learning: In 'Result.cs', the user prefers not to add additional data URI validation in the 'IcoPath' property setter.
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.
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs (1)
171-172
: LGTM: Path trimming logic correctly addresses the stated issue.The use of
TrimEnd('\\', '/')
appropriately removes trailing backslashes and forward slashes, which aligns with the PR objective to fix path type checking issues. The comments clearly explain the purpose.Also applies to: 187-188
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs
Outdated
Show resolved
Hide resolved
Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This PR addresses path type checking issues by ensuring that any trailing directory separators in the selected paths are removed.
- Update file selection functionality to trim trailing slashes
- Update folder selection functionality to trim trailing slashes
- Add the required System.IO reference for the new method usage
@@ -167,7 +168,8 @@ private void SelectPath_OnClick(object commandParameter, RoutedEventArgs e) | |||
string.IsNullOrEmpty(openFileDialog.FileName)) | |||
return; | |||
|
|||
SelectedPath = openFileDialog.FileName; |
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.
This seems to come from the windows dialogue control, why does it have slash for a filename path? Have you tested this resolves the issue?
not sure. I thought the reason is this because of that issue
…---Original---
From: "Jeremy ***@***.***>
Date: Fri, Jun 27, 2025 16:54 PM
To: ***@***.***>;
Cc: "Jack ***@***.******@***.***>;
Subject: Re: [Flow-Launcher/Flow.Launcher] Remove the last \\ or / in path ifit exists because it can cause path type checking issues (PR #3778)
@jjw24 commented on this pull request.
In Plugins/Flow.Launcher.Plugin.Explorer/Views/QuickAccessLinkSettings.xaml.cs:
> @@ -167,7 +168,8 @@ private void SelectPath_OnClick(object commandParameter, RoutedEventArgs e) string.IsNullOrEmpty(openFileDialog.FileName)) return; - SelectedPath = openFileDialog.FileName;
This seems to come from the windows dialogue control, why does it have slash for a filename path? Have you tested this resolves the issue?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
might be worth double checking, doesnt seem like it's the right fix. |
Remove the last \ or / in path if it exists because it can cause path type checking issues
Resolve #3777.