Conversation
|
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`.
blyxxyz
left a comment
There was a problem hiding this comment.
I'm interested in this and it looks good, are you willing to pick it up again?
| case_sensitive: true, | ||
| require_literal_separator: false, | ||
| require_literal_leading_dot: false, | ||
| follow_links: false, |
There was a problem hiding this comment.
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?
| let metadata = if follow_links { | ||
| fs::metadata(p) | ||
| } else { | ||
| fs::symlink_metadata(p) | ||
| }; |
There was a problem hiding this comment.
Does the fs::metadata() call in fill_todo() need the same treatment?
|
I'd be against having this in 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. |
|
@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? |
|
@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. |
|
@brmmm3 I'm not against having a follow symlinks option. I am against putting it there, in the |
|
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? |
Add
follow_linksoption inMatchOptionsstruct to signalizewhen to follow symlinks when the directory is traversed. The
default value is
false.