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

Add option for readdir to prepend the relative or absolute path #33004

Closed
wants to merge 3 commits into from
Closed

Add option for readdir to prepend the relative or absolute path #33004

wants to merge 3 commits into from

Conversation

tamasgal
Copy link

This is a simple implementation of the feature request created by @yakir12 in #29309 after discussing it on https://discourse.julialang.org/t/readdir-return-full-path/15274?u=yakir12

@tamasgal
Copy link
Author

Unfortunately I am not able to run the test suite (I get segmentation faults) so there are no unit tests yet until I figure out what's going on.

@StefanKarpinski
Copy link
Member

I'm not really into this change. The readdir function does one very simple thing: it reads the contents of a directory, which is a vector of the names in that directory. Yes, you can do all sorts of complicated stuff with that, but that's up to you.

@yakir12
Copy link

yakir12 commented Aug 22, 2019

So you feel that adding that option over-complicates readdir?

@tamasgal
Copy link
Author

Well, I have never used readdir() to just retrieve a list of filenames in a given directory. I always use it to directly work with the files (e.g. in mass processing), so I need the full path (relative or absolute, doesn't matter). I think that most of the people who liked that idea do the same.

I know that one can use Glob, but that would be an additional dependency.

@StefanKarpinski
Copy link
Member

So you feel that adding that option over-complicates readdir?

Yes. Note that the C function by the same name and the Python os.listdir function have the same behavior.

@tamasgal
Copy link
Author

OK, let's close this then!

@StefanKarpinski
Copy link
Member

Something about closing this kept nagging at me—after all, it is often more convenient to get joined paths or absolute paths. One of the things that bothers me about this proposed API is that it's non-orthogonal: if you set abspath=true then the join option no longer matters. So I've opened #33113 as an alternative that only introduces join and doesn't add the abspath option. Instead you can get absolute paths by passing an absolute directory name and asking for joined names.

@yakir12
Copy link

yakir12 commented Aug 29, 2019

Win win!

@tamasgal
Copy link
Author

Nice! I like #33113, I am totally fine with the join-functionality only ☺️

@rfourquet
Copy link
Member

Something about closing this kept nagging at me

Same for me and I wanted to look closer at this but you reacted faster, nice!

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 this pull request may close these issues.

4 participants