-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Split paths by both backslash and forward slash on Windows #505
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes made in the pull request involve updating the Changes
Assessment against linked issues
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: 0
Outside diff range and nitpick comments (2)
autoload/fern/internal/filepath.vim (2)
Line range hint
9-12
: Consider updatingfern#internal#filepath#from_slash
for Unix systemsThe
fern#internal#filepath#from_slash
function currently callss:to_slash_unix
for Unix systems, which might not be the intended behavior. Consider creating a dedicateds:from_slash_unix
function or verifying if the current implementation is correct.function! fern#internal#filepath#from_slash(path) abort return g:fern#internal#filepath#is_windows \ ? s:from_slash_windows(a:path) \ : s:from_slash_unix(a:path) " This function doesn't exist yet endfunction
Line range hint
48-51
: Consider updatings:from_slash_windows
for consistencyThe
s:to_slash_windows
function now handles both backslashes and forward slashes, buts:from_slash_windows
still only splits on forward slashes. For consistency, consider updatings:from_slash_windows
to handle both slash types as well:function! s:from_slash_windows(path) abort let terms = filter(split(a:path, '\\/'), '!empty(v:val)') let path = join(terms, '\') return path[:2] =~# '^\w:$' ? path . '\' : path endfunctionThis change would make the behavior more symmetric and robust.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- autoload/fern/internal/filepath.vim (1 hunks)
Additional comments not posted (1)
autoload/fern/internal/filepath.vim (1)
43-43
: Excellent modification to improve path handling on WindowsThis change effectively addresses the PR objective and the issue mentioned in #478. By modifying the
split
function to use both backslashes and forward slashes as delimiters, thes:to_slash_windows
function can now handle Windows paths regardless of the slash type used.Key improvements:
- Increased flexibility in path handling on Windows.
- Resolves the issue where the reveal function didn't work with forward slash separated paths.
- Maintains consistency with the function's purpose of standardizing path formats.
The change is minimal, targeted, and doesn't introduce any apparent negative side effects. Great job on implementing this solution!
0ea569c
to
473a94e
Compare
May close #478
Summary by CodeRabbit
New Features
Bug Fixes