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

Fail to rename a file with different case in Mac #484

Open
gnoeyp opened this issue Apr 25, 2023 · 6 comments · May be fixed by #485
Open

Fail to rename a file with different case in Mac #484

gnoeyp opened this issue Apr 25, 2023 · 6 comments · May be fixed by #485

Comments

@gnoeyp
Copy link

gnoeyp commented Apr 25, 2023

Path:rename() does not allow to rename "abc.txt" to "Abc.txt"

@miversen33
Copy link
Contributor

The mac filesystem is case insensitive so a rename of abc.txt to Abc.txt is nonsensical (to the OS) as they are the same.

@gnoeyp
Copy link
Author

gnoeyp commented Apr 25, 2023

abc.txt and Abc.txt can't exist in the same directory, but it is still possible to rename abc.txt to Abc.txt using Finder or terminal, if there is no duplication. I often need to rename a file in camel case to pascal case, or conversely. Is it not possible to add this feature?

@miversen33
Copy link
Contributor

miversen33 commented Apr 25, 2023

I don't know the work effort to do this, it would depend on if luv (the underlying lua tool that is used to interface with the host) supports this. This will have to wait for someone more intimately familiar with that section of the codebase. Can you share the error you are receiving when you attempt the rename? And can you verify that you can execute an mv abc.txt Abc.txt?

@gnoeyp
Copy link
Author

gnoeyp commented Apr 25, 2023

This is what I got. And I am sure that you can execute mv abc.txt Abc.txt in Mac (I tried just now).
스크린샷 2023-04-25 오후 11 36 35

@tmillr
Copy link

tmillr commented May 3, 2023

The mac filesystem is case insensitive so a rename of abc.txt to Abc.txt is nonsensical (to the OS) as they are the same.

File name comparisons are done without regard to case, however, the fs is also case-preserving, meaning that file names are stored and retrieved exactly how they were entered. This is generally the case for most modern/common "case-insensitive" filesystems I believe (unless you are on something like DOS, which is not a modern OS).

Just because the current host/fs has a case-insensitive fs doesn't mean that that will always be the case for a given repo/codebase. For example, if you were working on a shared codebase on macOS, it may become necessary at some point (hypothetically speaking) to rename a makefile to Makefile (as the former wouldn't work on case-sensitive file systems, even though it would on your own), etc. So there definitely seems to be a use-case for wanting to do such a thing, even on a case-insensitive fs.


The error is being thrown by Plenary itself in Path:rename().

I don't think the fix is trivial, nor could it be perfect and unsusceptible to edge-case race conditions, but I may give it a shot and send in a patch/pr 😄. I'm not super familiar with Plenary's code though, although it does seem that Path:rename() is missing type annotations and might have some unnecessary code in its impl. The reason the fix is probably not easy/simple is because:

  1. the current behavior of Path:rename() is to not allow clobbering/overwriting the new filename if it appears to already exist (and this differs from the semantics of the rename(2) syscall alone, which is the function that Path:rename() is ultimately using/wrapping to do the rename), and furthermore there is no option/arg that one could pass to "force" it
  2. I could be wrong, but there doesn't appear to be a trivial/universal/solid/builtin/canonical way to check for case-sensitivity (e.g. even mac/darwin can have case-sensitive filesystems, statfs() doesn't appear to give you that kind of info, etc.).

Therefore, the solution will probably end up being something rather manual and "hacky", if the current behavior as described in 1. is to be retained.

@tmillr
Copy link

tmillr commented May 3, 2023

I think I may have almost reached a solution.

On an unrelated note, I also came across 2 or 3 small bugs in Path:rename() as well (that have to do with using stat instead of lstat, or extracting the wrong substring causing a newpath such as ./* to throw). I'll try to fix those too.

@tmillr tmillr linked a pull request May 3, 2023 that will close this issue
2 tasks
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 a pull request may close this issue.

3 participants