-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add File.same?(path1, path2) with follow_symlinks option #6161
Conversation
05b4431
to
4cf4ed8
Compare
@@ -54,7 +54,7 @@ struct Crystal::System::FileInfo < ::File::Info | |||
@stat.st_gid.to_u32 | |||
end | |||
|
|||
def ==(other : ::File::Info) : Bool | |||
def same_file?(other : ::File::Info) : Bool |
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.
Can we add a File#==
as well, which does info on both file instances and then same_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.
I don't think the semantic of File#==
to compare files is accurate. The file has more state that the file description.
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.
It shouldn't do. But I guess this is fine for now...
@@ -54,7 +54,7 @@ struct Crystal::System::FileInfo < ::File::Info | |||
@stat.st_gid.to_u32 | |||
end | |||
|
|||
def ==(other : ::File::Info) : Bool | |||
def same_file?(other : ::File::Info) : Bool |
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 now needs an abstract def
in src/file/info.cr
.
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.
Updated in the next commit
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.
Allright!
I'm just not sure whether follow_symlinks
should be true by default. I would tend to false myself.
@ysbaddaden I agree, but the rest of the defaults for follow_symlink was true. I thought it was better to keep the same default. |
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.
I wonder the same about follow_symlinks
, but it's not a huge deal so...
Ok, I've changed the default to not follow symlinks. |
Follow up to #5584
Add
File.same?(path1, path2)
method to check if path refers to same file.Rename the
FileInfo#==
toFileInfo#same_file?
to have better semantics of comparing inodes.