feat: add FilesFromFS #62
Conversation
Add function to map a fs.FS into a list of FileInfo entries. Fixes mholt#61 Signed-off-by: Joonas Bergius <joonas@bergi.us>
mholt
left a comment
mholt
left a comment
There was a problem hiding this comment.
Thanks! Sorry for the delay...
I think this looks good, let's just make sure the comments make sense in the context of the FS instead of the OS file system.
archives.go
Outdated
| // Map values should use slash ('/') as the separator regardless of | ||
| // the platform, as most archive formats standardize on that rune as the | ||
| // directory separator for filenames within an archive. For convenience, map |
There was a problem hiding this comment.
This comment is probably irrelevant, or at least should be reworded, for this context, since all fs.FS paths use / as path separators.
There was a problem hiding this comment.
Good catch, I reworded a bit, please let me know if you think you'd like to clarify it further.
archives.go
Outdated
| // which is expected to be prefixed by rootOnDisk (according to fs.WalkDirFunc godoc) | ||
| // and which will be placed into a folder rootInArchive in the archive. | ||
| func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive string) string { | ||
| func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive, separator string) string { |
There was a problem hiding this comment.
| func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive, separator string) string { | |
| func nameOnDiskToNameInArchive(nameOnDisk, rootOnDisk, rootInArchive string, separator rune) string { |
Thinking the separator should be a rune to keep it aligned with the actual values of filepath.Separator.
There was a problem hiding this comment.
Makes sense, I went back and forth originally as to whether to use rune or string here, I changed it back to rune.
|
As for:
I'm OK with not supporting older versions of Go... especially a version that is almost a year old. I think if anyone needs the new feature they can upgrade to the new version. |
Signed-off-by: Joonas Bergius <joonas@bergi.us>
…arator values Signed-off-by: Joonas Bergius <joonas@bergi.us> Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
Just to double check my understanding, are you suggesting one of these approaches:
I'm assuming you meant option 1 from above, but I figured I'd double check before I went ahead and implemented. |
|
Right, thanks -- to clarify, I was thinking the first option (bump up the minimum version). |
Signed-off-by: Joonas Bergius <joonas@bergi.us>
Signed-off-by: Joonas Bergius <joonas@bergi.us>
2 participants
Add function to map a
fs.FSinto a list ofarchives.FileInfoentries.Fixes #61
The open question that remains is whether symlinks should be supported?
As best as I can tell, this is possible as of Go 1.25 when
fs.ReadLinkFSis implemented by the FS, but that leaves the question about what should be done for Go 1.24 and below where this functionality does not exist?For this initial pass, I opted to return an error if the function comes across a symlink on the provided
fs.FSso we can talk about what is the appropriate way to handle this.