-
Notifications
You must be signed in to change notification settings - Fork 92
Conversation
aplanas
commented
Sep 20, 2017
Initial proposal for issue #62
Add `follow_links` option in `MatchOptions` struct to signalize when to follow symlinks when the directory is traversed. The default value is `false`.
16d166a to
76515ba
Compare
@blyxxyz
blyxxyz
left a comment
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'm interested in this and it looks good, are you willing to pick it up again?
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 think it's the right call (it matches all shells I tried as well as Python's glob module) but a default of false would be a breaking change, right?
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.
Does the fs::metadata() call in fill_todo() need the same treatment?
piegamesde
commented
Nov 12, 2021
I'd be against having this in MatchOptions. Glob matching should always be a comparison against strings. No file system should be harmed in the process :)
As far as I can tell, the symlink problem arises from the helper methods that walk the directory structure in some way. They should have their own separate option system instead.
aplanas
commented
Nov 12, 2021
@piegamesde Oh that is a good argument. I forgot this PR actually, but where do you think that the "follow_symlink" flag should live? As a boolean parameter is some function?
brmmm3
commented
Apr 15, 2023
@piegamesde I don't really understand your argument against this option. If I traverse a file tree and want to get, or check, the symlinks themselves then I cannot use glob without the follow_symlink option. I would appreciate to have this in included. To not change the default behavior the default value of this option can be true.
@aplanas Please can you consider to merge this with follow_symlink=true as the default value?
piegamesde
commented
Apr 15, 2023
@brmmm3 I'm not against having a follow symlinks option. I am against putting it there, in the MatchOptions. The reason is that MatchOptions is also used by functions like Pattern::matches_with, which do not and should not involve any file system operations. Having a file system related option in that struct would be confusing and unfit.
JohnTitor
commented
Apr 22, 2023
While @piegamesde's argument is valid, I also think this is the simplest way. Maybe we can add a note describing this option doesn't make sense in some context?
Uh oh!
There was an error while loading. Please reload this page.
Add
follow_linksoption inMatchOptionsstruct to signalizewhen to follow symlinks when the directory is traversed. The
default value is
false.