Add ListParser as a config option #38
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
muirmanders
requested changes
Jul 20, 2019
| @@ -149,6 +150,12 @@ type Config struct { | |||
| // hung connections. | |||
| DisableEPSV bool | |||
|
|
|||
| // Provide a custom parser for the outcome of the LIST command | |||
| // If this is not set, a default LIST parser is used that will handle most outcomes | |||
| ListParser interface { | |||
There was a problem hiding this comment.
Can this just be a function instead of an interface?
| @@ -139,6 +139,9 @@ func (c *Client) ReadDir(path string) ([]os.FileInfo, error) { | |||
| return nil, err | |||
| } | |||
| parser = func(entry string, skipSelfParent bool) (os.FileInfo, error) { | |||
| if c.config.ListParser != nil { | |||
| return c.config.ListParser.ParseLIST(entry) | |||
There was a problem hiding this comment.
What about the skipSelfParent flag? (I think its purpose is to exclude "." and ".." when listing.) Perhaps instead of passing that flag into the parser, we could exclude "." and ".." here using the return value?
| @@ -149,6 +150,12 @@ type Config struct { | |||
| // hung connections. | |||
| DisableEPSV bool | |||
|
|
|||
| // Provide a custom parser for the outcome of the LIST command | |||
| // If this is not set, a default LIST parser is used that will handle most outcomes | |||
| ListParser interface { | |||
There was a problem hiding this comment.
Can you add tests for the new functionality?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've encountered some cases where the default parseLIST function fails to parse the outcome of a LIST command, despite getting useful data. This was encountered while communicating with an older FTP server.
Rather than attempting to cover all the types of output an FTP server could give in response to a LIST command, I've added the ability to specify a custom ListParser to handle these cases.
Here's a sample format i've seen that could not be handled, with fake fields:
OWNER 777 11/12/13 01:02:03 TYPE /some/directory/file.txt