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

walkdir library #104

Closed
zhangkaizhao opened this issue May 28, 2019 · 7 comments
Closed

walkdir library #104

zhangkaizhao opened this issue May 28, 2019 · 7 comments

Comments

@zhangkaizhao
Copy link

zhangkaizhao commented May 28, 2019

Implementation in other popular languages:

walkdir_test: https://github.com/lilydjwg/walkdir-test

@watzon
Copy link

watzon commented Jun 15, 2019

What's wrong with Dir? What are you wanting the shard to be able to do?

Currently with Dir you can do:

Dir["**/*"].each do |f|
  puts f
end

@zhangkaizhao
Copy link
Author

Hi @watzon ! Please read at least one of the implementations e.g. Python I referred, and find if it is already implemented in Crystal. Please update me if I missed something. Thank you!

@watzon
Copy link

watzon commented Jun 18, 2019

@zhangkaizhao I have looked through all of the implementations and I haven't seen much of anything that can't be done with plain Crystal. That's why I want to know what your use-case is. What are you wanting to do that you can do in one of those libs, but not with Dir?

@zhangkaizhao
Copy link
Author

@watzon Thanks! After some more investigation I think you are right. I made a mistake.
This Dir["**/*"].each is a little magical. 😅
I am closing this issue now.
And I will try to add the implementation in Crystal for the walkdir_test project soon.

@zhangkaizhao
Copy link
Author

zhangkaizhao commented Jun 20, 2019

This is my pull request lilydjwg/walkdir-test#1 .
The class method Dir.[](*patterns) : Array(String) returns an Array. This might be an issue when there are too many files in the directory.
You can improve it if you have a better idea.

Without this method, I have to open sub directories recursively:

def count(dir)
  count = 0
  dir.each_child do |filename|
    subpath = File.join dir.path, filename
    filetype = File.info(subpath, follow_symlinks: false).type
    if filetype.directory?
      count += count(Dir.new(subpath))
    elsif filetype.file? && filename.downcase.ends_with?(".jpg")
      count += 1
    end
  end
  dir.close
  count
end

puts count(Dir.new(ARGV[0]))

and this maybe hits the recursion limit for too deep sub directories.

And another problem is when I run without the line dir.close from the above source code I get an issue which complains of "Too many open files" if the number of files is a little bigger:

$ ./walk ~/repos/github.com/
Unhandled exception: Error opening directory '/home/kaizhao/repos/github.com/ziglang/zig.wiki/.git/logs/refs': Too many open files (Errno)
Failed to raise an exception: END_OF_STACK
[0x563f100af6b6] ???
[0x563f10096edb] __crystal_raise +43
[0x563f10097e6c] ???
[0x563f1009f380] ???
[0x563f1009a94c] ???
[0x563f10099a3d] ???
[0x563f100a2787] ???
[0x563f100aaf4c] ???
[0x563f10098e97] main +55
[0x7f7f5edf9ee3] __libc_start_main +243
[0x563f100954fe] _start +46
[0x0] ???

I don't know if this is an issue.

@bew
Copy link

bew commented Jun 20, 2019

The class method Dir. : Array(String) returns an Array. This might be an issue when there are too many files in the directory.

You can use Dir.glob with a block if you want to avoid creating an array

zhangkaizhao added a commit to zhangkaizhao/walkdir-test that referenced this issue Jun 20, 2019
Use `Dir.glob` instead of `Dir.[]` to avoid creating an array.
This was suggested by @bew in a comment [1].

[1]: crystal-community/crystal-libraries-needed#104 (comment)
@zhangkaizhao
Copy link
Author

@brew Thank you! I have updated the pull request with Dir.glob.

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

No branches or pull requests

3 participants