Add support for path beginning with parent directory (..) #5
Conversation
single_include/glob/glob.hpp
Outdated
| bool is_hidden(const std::string &pathname) { | ||
| // path is hidden if it starts with dot and is not parent dir nor current dir | ||
| if (pathname.size() >= 2) | ||
| return pathname.at(0) == '.' && pathname.at(1) != '.' && pathname.at(1) != '/'; | ||
| return pathname[0] == '.'; | ||
| } |
There was a problem hiding this comment.
What should be the result of is_hidden("../.foobar")? This implementation would return false.
There was a problem hiding this comment.
Hi,
indeed I agree that my pretty basic implementation would fail. However to be fair, the actual implementation would also fail, returning false for any path that starts with . such as for example ../foo or ./bar, leading to the path not being searched at all using the glob/rglob functions. I wanted a very simple and nonintrusive implementation that fit my needs of globing parent paths but I'm very open to alternatives.
I personally think that the function should look for any directory that is hidden within the given path and not only the first character. If any of those directories begins with dot and followed by something else than dot or slash, then the function should consider this path as hidden.
Also but unrelated, unless there is verification somewhere else, is_hidden will crash if given an empty string as parameter (line 188 accesses index 0 without checking if pathname is empty).
BTW, thanks for the library, very useful and simple to integrate!
There was a problem hiding this comment.
Fully agree with your points and support the argument that this needs to be fixed.
I was just wondering what should be the right result in the imagined scenarios - the scenario I imagined here is "../.foobar", i.e., a hidden file in a parent directory. A good place to start is to probably see what Python's implementation does in this regard.
I really want to accept this PR but maybe we can add (1) the check for empty string, and (2) consider the case I have imagined?
There was a problem hiding this comment.
Agreed. I'll rework my PR to support those.
|
Hi again, What do you think? |
|
Looks great! Thanks!! |
This PR adds support for paths starting with the parent directory (..). It modifies the is_hidden() function so that paths starting with parent directory ( ../ ) or current directory ( ./ ) should not be considered hidden.