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

NERDTreeFind: Exception not caught: NERDTree.InvalidArgumentsError: <file> should be under <path> #1344

Closed
dangibson opened this issue Jan 26, 2023 · 8 comments · Fixed by #1387
Labels

Comments

@dangibson
Copy link
Contributor

Windows, NERDTree 6.10.16
Open a folder in NERDTree, c:\dev. This has a sub folder called Project - ie c:\dev\Project.
Open a file in vim, c:\dev\project\file.txt
Note the path is called "c:\dev\Project" while the file is opened as "c:\dev\project\file.txt" - casing is the issue here.
Run command NERDTreeFind

You get an error:
Error detected while processing function 28_findAndRevealPath[47]....
E605: Exception not caught: NERDTree.InvalidArgumentsError: c:\dev\project\file.txt should be under c:\dev\Project

I'll submit a pull request to fix this.

@dangibson dangibson added the bug label Jan 26, 2023
dangibson added a commit to dangibson/nerdtree that referenced this issue Jan 26, 2023
…alidArgumentsError: <file> should be under <path>
dangibson added a commit to dangibson/nerdtree that referenced this issue Jan 26, 2023
…alidArgumentsError: <file> should be under <path>
@rzvxa
Copy link
Member

rzvxa commented Oct 18, 2023

@dangibson
Hi,
May I ask why you closed your PR?
I've recently submitted a PR to solve another problem with case-insensitive file systems when using the move operation(#1375), and introduced a flag called g:NERDTreeCaseInsensitiveFS instead of guessing based on the operating system as both macOS and Windows can support case sensitive file systems.
I think with the use of this flag we can also fix this problem across platforms instead of specifically on Windows machines.
Would you like to reopen your PR and use this new flag to solve this issue? Even if you don't want to work on it anymore I will create a PR based on your commits to keep the credits for them.
Let me know if you want to continue working on it.

@dangibson
Copy link
Contributor Author

I fixed the issue I had and it's been working fine for me. I didn't think I closed it.

@rzvxa
Copy link
Member

rzvxa commented Oct 18, 2023

Oh, I don't know why I thought it was closed, what do you think about using the same flag in your PR? do you think it's a good idea?

@dangibson
Copy link
Contributor Author

I would probably default to assuming it is case insensitive instead of sensitive. While that is technically wrong for linux, it's unlikely people would have a file or folder called "abc" and another called "Abc" in the same folder. Although the end result is the same, the difference is that on Windows you wouldn't have to set the flag manually, and on linux you probably wouldn't need to change it so it would be a slightly better out of the box experience.

If you have a pull request that resolves the issue then I'm happy to scrap mine - no credit needed. I resolved the issue for me and submitted a PR but nothing happened with it and I don't really have an interest in working further on it.

@rzvxa
Copy link
Member

rzvxa commented Oct 18, 2023

I've set it to Case sensitive by default to make it backward compatible, this way we don't break anything for users who are used to this behavior and may have put their workspace(although stupid) around this thing.
You can have something like this in your .vimrc file if you work with the same vim configuration on different machines

 if nerdtree#runningWindows()
     let g:NERDTreeCaseInsensitiveFS = 1
 elseif has('gui_mac') || has('gui_macvim') || has('mac') || has('osx')
     let g:NERDTreeCaseInsensitiveFS = 1
 else
     let g:NERDTreeCaseInsensitiveFS = 0
 endif

If we assume case sensitivity wrong in critical places it can cause files to be overwritten which is not a good thing to happen,
Not showing some files or throwing an error is much safer than overwriting it that's why I thought it is much better to give the choice to the end user. We can reuse this flag for many purposes, And in the future, we have the option to turn it on by default for Windows and macOS as they have case-insensitive file systems by default.

Let me know what you think about it.

@dangibson
Copy link
Contributor Author

I think on Windows / mac it should default to insensitive without the user needing to explicitly set it since that is what the users would reasonably expect to happen. If it defaults to sensitive then they would consider it a bug and spend time searching for the solution.
I think it is better to set the flag now instead of 'in the future' since 'in the future' there would be resistance to changing it since it wouldn't be backwards compatible.

@rzvxa
Copy link
Member

rzvxa commented Oct 18, 2023

I don't have a strong take on whether or not this flag should be on by default or not, But I think you agree with the fact that this flag is needed in the first place, Right?

@dangibson
Copy link
Contributor Author

The flag is a good idea - testing a flag is easier than testing is windows or is osx or is mac or is macvim etc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants