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

fs: standard implementations of FileInfo and DirEntry should implement fmt.Stringer #54451

Closed
dsnet opened this issue Aug 15, 2022 · 12 comments
Closed

Comments

@dsnet
Copy link
Member

dsnet commented Aug 15, 2022

2023-04-12: The current proposal is #54451 (comment).


For debugging purposes, one may want to print a FileInfo to see high-level information about the file without manually formatting each field in the interface (e.g., name, mode, modification date, size, etc.).

For entries returned by os, the native representation is nicely printed by fmt:

go/src/os/file_unix.go

Lines 394 to 399 in 03fb5d7

type unixDirent struct {
parent string
name string
typ FileMode
info FileInfo
}

However, the embed.file type has the following structure:

go/src/embed/embed.go

Lines 218 to 224 in 03fb5d7

type file struct {
// The compiler knows the layout of this struct.
// See cmd/compile/internal/staticdata's WriteEmbed.
name string
data string
hash [16]byte // truncated SHA256 hash
}

where printing a embed.file results in a potentially massive output being printed due to the entire file being contained in embed.file.data.

I propose we have all standard implementations (or just those that print poorly) of fs.FileInfo and fs.DirEntry to implement a String method that prints useful information.

One possible default formatting is something similar to what the Unix ls command prints:

return fmt.Sprintf("%v %d %v %s", fi.Mode(), fi.Size(), fi.ModTime().Round(0), fi.Name())
@dsnet dsnet added the Proposal label Aug 15, 2022
@gopherbot gopherbot added this to the Proposal milestone Aug 15, 2022
@robpike
Copy link
Contributor

robpike commented Aug 15, 2022

SGTM but please also propose just what it should print.

@rsc
Copy link
Contributor

rsc commented Mar 15, 2023

The output would look a bit like ls -l: -rw-r--r-- 8160 2023-03-15 15:04:05 UTC /etc/passwd.
That seems reasonable and recognizable.

@rsc
Copy link
Contributor

rsc commented Mar 15, 2023

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

If all our FileInfo and DirEntry implementations are going to add a String method, we should also provide helpers that all those implementations can call. Those would probably be:

func FileInfoString(info FileInfo) string
func DirEntryString(dir DirEntry) string

and then implementations such as os.fileStat would do

func (s *fileStat) String() string { return fs.FileInfoString(s) }

It looks like these are the types affected by this proposal, which would all add String methods:

% git grep -n ') ModTime('
archive/tar/common.go:549:func (fi headerFileInfo) ModTime() time.Time { return fi.h.ModTime }
archive/zip/reader.go:761:func (f *fileListEntry) ModTime() time.Time {
archive/zip/struct.go:181:func (fi headerFileInfo) ModTime() time.Time {
archive/zip/struct.go:275:func (h *FileHeader) ModTime() time.Time {
cmd/distpack/archive.go:45:func (i fileInfo) ModTime() time.Time { return i.f.Time }
cmd/go/internal/fsys/fsys.go:582:func (f fakeFile) ModTime() time.Time { return f.real.ModTime() }
cmd/go/internal/fsys/fsys.go:595:func (f missingFile) ModTime() time.Time { return time.Unix(0, 0) }
cmd/go/internal/fsys/fsys.go:607:func (f fakeDir) ModTime() time.Time { return time.Unix(0, 0) }
cmd/go/internal/modfetch/coderepo.go:1154:func (fi dataFileInfo) ModTime() time.Time { return time.Time{} }
cmd/pack/pack_test.go:488:func (f *FakeFile) ModTime() time.Time {
embed/embed.go:233:func (f *file) ModTime() time.Time         { return time.Time{} }
net/http/fs_test.go:778:func (f *fakeFileInfo) ModTime() time.Time { return f.modtime }
os/types_plan9.go:23:func (fs *fileStat) ModTime() time.Time { return fs.modTime }
os/types_unix.go:25:func (fs *fileStat) ModTime() time.Time { return fs.modTime }
os/types_windows.go:168:func (fs *fileStat) ModTime() time.Time {
testing/fstest/mapfs.go:157:func (i *mapFileInfo) ModTime() time.Time         { return i.f.ModTime }

% git grep -n ') Info('
archive/zip/reader.go:768:func (f *fileListEntry) Info() (fs.FileInfo, error) { return f, nil }
archive/zip/struct.go:191:func (fi headerFileInfo) Info() (fs.FileInfo, error) { return fi, nil }
cmd/compile/internal/types2/basic.go:76:func (b *Basic) Info() BasicInfo { return b.info }
cmd/distpack/archive.go:35:func (f *File) Info() fs.FileInfo {
cmd/gofmt/long_test.go:181:func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil }
embed/embed.go:237:func (f *file) Info() (fs.FileInfo, error) { return f, nil }
go/types/basic.go:78:func (b *Basic) Info() BasicInfo { return b.info }
io/fs/readdir.go:62:func (di dirInfo) Info() (FileInfo, error) {
io/fs/walk.go:137:func (d *statDirEntry) Info() (FileInfo, error) { return d.info, nil }
log/syslog/syslog.go:238:func (w *Writer) Info(m string) error {
os/dir_plan9.go:81:func (de dirEntry) Info() (FileInfo, error) { return de.fs, nil }
os/dir_windows.go:142:func (de dirEntry) Info() (FileInfo, error) { return de.fs, nil }
os/file_unix.go:425:func (d *unixDirent) Info() (FileInfo, error) {
path/filepath/path.go:554:func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil }
path/filepath/path_test.go:571:func (d *statDirEntry) Info() (fs.FileInfo, error) { return d.info, nil }
syscall/exec_unix_test.go:28:func (c *command) Info() (pid, pgrp int) {
testing/fstest/mapfs.go:160:func (i *mapFileInfo) Info() (fs.FileInfo, error) { return i, nil }

@rsc
Copy link
Contributor

rsc commented Apr 5, 2023

To summarize, the proposal is:

(1) Add

func FormatFileInfo(info FileInfo) string
func FormatDirEntry(dir DirEntry) string

to io/fs for use by any implementations that want them.

(2) For all implementations in the standard library, define String methods calling those helpers.

Do I have that right? Have all concerns about this been addressed?

Updated 2023-04-12: changed names to FormatFileInfo and FormatDirEntry (formerly FileInfoString and DirEntryString).

@dsnet
Copy link
Member Author

dsnet commented Apr 5, 2023

SGTM. To bikeshed, maybe call it FormatFileInfo to match strconv.FormatXXX?

@rsc
Copy link
Contributor

rsc commented Apr 12, 2023

Sure.

@rsc
Copy link
Contributor

rsc commented Apr 12, 2023

Based on the discussion above, this proposal seems like a likely accept.
— rsc for the proposal review group

@rsc
Copy link
Contributor

rsc commented Apr 19, 2023

No change in consensus, so accepted. 🎉
This issue now tracks the work of implementing the proposal.
— rsc for the proposal review group

@rsc rsc changed the title proposal: fs: standard implementations of FileInfo and DirEntry should implement fmt.Stringer fs: standard implementations of FileInfo and DirEntry should implement fmt.Stringer Apr 19, 2023
@rsc rsc modified the milestones: Proposal, Backlog Apr 19, 2023
@gopherbot
Copy link

Change https://go.dev/cl/489555 mentions this issue: io/fs: add FormatFileInfo and FormatDirEntry functions

@gopherbot
Copy link

Change https://go.dev/cl/491175 mentions this issue: all: add String for fs.{FileInfo,DirEntry} implementations

gopherbot pushed a commit that referenced this issue May 2, 2023
For #54451

Change-Id: I3214066f77b1398ac1f2786ea035c83f32f0a826
Reviewed-on: https://go-review.googlesource.com/c/go/+/489555
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Joseph Tsai <joetsai@digital-static.net>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@gopherbot
Copy link

Change https://go.dev/cl/499177 mentions this issue: doc/go1.21: document io/fs formatting functions

gopherbot pushed a commit that referenced this issue May 31, 2023
Also document the new String methods that call them.

For #54451

Change-Id: I5cd7e0fc6c84097bba6d29c4d6012ed3c8bb1e0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/499177
Reviewed-by: Eli Bendersky <eliben@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@dmitshur dmitshur modified the milestones: Backlog, Go1.21 Jun 4, 2023
Nasfame pushed a commit to golangFame/go that referenced this issue Jun 4, 2023
Also document the new String methods that call them.

For golang#54451

Change-Id: I5cd7e0fc6c84097bba6d29c4d6012ed3c8bb1e0d
Reviewed-on: https://go-review.googlesource.com/c/go/+/499177
Reviewed-by: Eli Bendersky <eliben@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Bypass: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Accepted
Development

No branches or pull requests

5 participants