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

support positive count in Readdir() #61

Merged
merged 11 commits into from Mar 20, 2019

Conversation

Projects
None yet
4 participants
@Songmu
Copy link
Contributor

commented Mar 4, 2019

follow on #60.

The Readdir() should support positive count in order to return the specified counts of files.

For it, I defined statikFS.dirs and httpFile.dirIdx for managing the internal state and use it inside Readdir().

@Songmu Songmu force-pushed the Songmu:readdir branch from 5ec7344 to 965c6f9 Mar 4, 2019

@mattn

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2019

I confirmed this pull-request fixes both problem. Current implementation of Readdir return all of files in the directory.

image

About problem that Name() return absolute path, this is not a big problem since most of users does not call Name() in the fs. If someone got this bug, they had report already. @rakyll I'm looking for fixing this issue. Could you please take a look?

@Songmu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 13, 2019

How about it? @rakyll

Show resolved Hide resolved fs/fs.go Outdated
Show resolved Hide resolved fs/fs.go Outdated
Show resolved Hide resolved fs/fs.go
Show resolved Hide resolved fs/fs_test.go Outdated
Show resolved Hide resolved fs/fs_test.go Outdated
Show resolved Hide resolved fs/fs.go

Songmu added some commits Mar 14, 2019

corresponds to the part pointed out in the review.
Fix error messages and variable names.
@Songmu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

Thank you for the kind reviews.
It corresponded to all the pointed out matters.
Please review again.
@rakyll

@dkumor

This comment has been minimized.

Copy link

commented Mar 20, 2019

@Songmu - as I understand, this PR encompasses the fixes in #55, #59 and #60, and meaning that those PRs can be closed once this gets merged in?

@rakyll

rakyll approved these changes Mar 20, 2019

@rakyll

This comment has been minimized.

Copy link
Owner

commented Mar 20, 2019

Thanks much.

@rakyll rakyll merged commit 3bac566 into rakyll:master Mar 20, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

This was referenced Mar 20, 2019

@Songmu Songmu deleted the Songmu:readdir branch Mar 21, 2019

@Songmu

This comment has been minimized.

Copy link
Contributor Author

commented Mar 23, 2019

Thank you, too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.