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

WithHookReadDirectory fs param is not useful #87

Closed
plastikfan opened this issue Jul 18, 2024 · 1 comment · Fixed by #90
Closed

WithHookReadDirectory fs param is not useful #87

plastikfan opened this issue Jul 18, 2024 · 1 comment · Fixed by #90
Assignees
Labels
bug Something isn't working

Comments

@plastikfan
Copy link
Contributor

plastikfan commented Jul 18, 2024

	tv.WithHookReadDirectory(func(_ fs.FS, dirname string) ([]fs.DirEntry, error) {
		return vfs.ReadDir(helpers.TrimRoot(dirname))
	}),

the above is an example of using this With hook, but the first parameter passed into the function, _ fs.FS is not not useful in the context within which it is being provided. Typically the client would want to read from the file system, but the incoming parameter of type fs.FS is a very narrow interface that only allows opening a file, so not useful. We need to either change this type to provided the ability to read from the file system, or remove it entirely.

Actually, Open might work to read the directory, so it needs to be verified; Open is documented as:

	// Open opens the named file.
	//
	// When Open returns an error, it should be of type *PathError
	// with the Op field set to "open", the Path field set to name,
	// and the Err field describing the problem.
	//
	// Open should reject attempts to open names that do not satisfy
	// ValidPath(name), returning a *PathError with Err set to
	// ErrInvalid or ErrNotExist.
	Open(name string) (File, error)

Actually, nah this is not right, since Open returns as fsFile, but we need to return a slice of fs.DirEntry.

Its looking like our read hook is defined incorrectly and should be modified:

func read(sys fs.FS, o *readOptions, path string) (*Contents, error)

!!!! You need to change this to work with fs.ReadDirFS:

func read(sys fs.ReadDirFS, o *readOptions, path string) (*Contents, error)

@plastikfan plastikfan added the bug Something isn't working label Jul 18, 2024
@plastikfan plastikfan self-assigned this Jul 18, 2024
@plastikfan
Copy link
Contributor Author

Fixing WithHookReadDirectory has now revealled an inconsistency with WithHookQueryStatus whose hook function signature does not provide a file system interface at all. It should probably take an extra parameter StatFS.

Currently tv.WithHookQueryStatus usage looks like this:

	tv.WithHookQueryStatus(func(path string) (fs.FileInfo, error) {
		return vfs.Stat(helpers.TrimRoot(path))
	}),

for this to work, we rely on the fact that still have access to vfs which is in itself not a bad thing, but it is not consistent with the way tv.WithHookReadDirectory now works.

The hook function tv.WithHookQueryStatus should take an extra StatFS parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant