Skip to content
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

Case Sensitive Move Operation #1375

Merged
merged 12 commits into from
Oct 22, 2023
Merged

Conversation

rzvxa
Copy link
Member

@rzvxa rzvxa commented Oct 17, 2023

Description of Changes

Closes #1373 and potentially many unresolved issues on the subject of case insensitivity.
In this commit, I've introduced a new option called NERDTreeCaseInsensitiveFS which could also be used in the future to resolve similar issues in other places.
Until now the Move Operation used to check if the destination path already exists or not, On case-insensitive file systems this check fails if the user wants to move the file to a location with a different casing (for example moving/renaming foo.bar to FOO.BAR) as both paths point to the same file. With this new option, we can have an extra check to see whether or not the new destination is the same path in a different casing, and if that's the case we can ignore this false positive check.
With this new change came a wrong artifact, After the rename operation we check if an open buffer to this path exists or not.
Then we will point every window to that buffer to a new buffer for the new file path, vim doesn't acknowledge the new file path if it already has one open for the old casing file, I've used a temp buffer to compensate for this issue. Now we first redirect every window to a temp buffer, close the old buffer, and point windows looking at the temp buffer to this newly created one.
This part is written very clearly and has a lot of comments explaining each step of the process in the code itself.


New Version Info

@alerque Sadly we skipped a few commits versioning, I would like to make a PR adding versions / CHANGELOG to the old merged PRs.
We can either block this PR until we fix the change log, or go ahead and merge it and give it a change log in the fix PR.

Author's Instructions

  • Derive a new MAJOR.MINOR.PATCH version number. Increment the:
    • MAJOR version when you make incompatible API changes
    • MINOR version when you add functionality in a backwards-compatible manner
    • PATCH version when you make backwards-compatible bug fixes
  • Update CHANGELOG.md, following the established pattern.

Collaborator's Instructions

  • Review CHANGELOG.md, suggesting a different version number if necessary.
  • After merging, tag the commit using these (Mac-compatible) bash commands:
    git checkout master
    git pull
    sed -n "$(grep -n -m2 '####' CHANGELOG.md | cut -f1 -d: | sed 'N;s/\n/,/')p" CHANGELOG.md | sed '$d'
    git tag -a $(read -p "Tag Name: " tag;echo $tag) -m"$(git show --quiet --pretty=%s)";git push origin --tags

alerque
alerque previously approved these changes Oct 18, 2023
alerque
alerque previously approved these changes Oct 20, 2023
@rzvxa rzvxa marked this pull request as draft October 20, 2023 20:43
@rzvxa rzvxa marked this pull request as ready for review October 21, 2023 04:44
@rzvxa
Copy link
Member Author

rzvxa commented Oct 21, 2023

I've gone through some iterations and tried to streamline our approach to potentially fix other similar issues using it.
I can confirm it works as intended on Windows, But it affects Mac users the most as there were some case-insensitive checks in place for Windows but not for macOS, and with this PR I've unified both operating systems under the new setting g:NERDTreeCaseSensitiveFS

@kevinkowalew
Copy link

I tested both 0 and 3 on macOS this morning and it achieves the desired behavior, should be good to merge 🎉

@alerque
Copy link
Member

alerque commented Oct 21, 2023

@kevinkowalew Thanks for testing and reporting, that's super helpful to move things forward!

@rzvxa
Copy link
Member Author

rzvxa commented Oct 22, 2023

Thanks to @kevinkowalew we have confirmation that these changes work on the macOS, I would've waited for @dangibson to give some feedback on this PR because he's also working on a similar issue in #1346 but since these changes are pretty essential and minor implementation details can always be changed in the subsequent patches I'm going to move forward with merging it into the upstream.

@rzvxa rzvxa merged commit ea4833d into preservim:master Oct 22, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(m)ove the current node shortcut doesn't treat filepaths as case sensitive
3 participants