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

proposal: archive/zip: *File should implement fmt.Stringer #23260

Closed
alandonovan opened this issue Dec 27, 2017 · 9 comments
Closed

proposal: archive/zip: *File should implement fmt.Stringer #23260

alandonovan opened this issue Dec 27, 2017 · 9 comments

Comments

@alandonovan
Copy link
Contributor

@alandonovan alandonovan commented Dec 27, 2017

I recently made the mistake of printing a *zip.File while debugging some zip reader code, expecting it to show me the name and perhaps the mode of the zip entry in some helpful form. It did show me these things, because fmt printed the raw struct, they were followed by masses of decimal numbers---the value of every byte in the file.

	rd, err := zip.NewReader(...)
	if err != nil { ... }
	for _, fh := range rd.File {
		fmt.Println(fh)
		...
	}

Here's one of the smaller lines of output:

&{{tools-3d92dd60033c312e3ae7cac319c792271cf67e37/cmd/goyacc/yacc.go 0 10 0 8 24966 18823 2447150300 21770 71391 21770 71391 [85 84 5 0 1 29 109 72 88] 0 } 0xc42025ca80 0xc420aa80c0 2525793 115491}

Ideally *File would have a friendly String method.

@odeke-em
Copy link
Member

@odeke-em odeke-em commented Dec 27, 2017

/cc @dsnet

@dsnet
Copy link
Member

@dsnet dsnet commented Dec 27, 2017

Not that I'm opposed to a String method:

  • What would the string print for zip.FileHeader?
  • Is String a precedence we want to set for other packages as well? (e.g., should tar.Header have a String method as well?)
@robpike
Copy link
Contributor

@robpike robpike commented Dec 28, 2017

The problem with "friendly String methods" for complex types, especially in core packages, is that different people have different definitions of what that might mean. In this case I think it's better to leave it as and have you print the things you're interested in.

@judepereira
Copy link

@judepereira judepereira commented Dec 29, 2017

@robpike - I think it'd be useful as it is, minus the every byte being printed, i.e. name and compression level.

@ericlagergren
Copy link
Contributor

@ericlagergren ericlagergren commented Dec 30, 2017

@judepereira that might be something to open a Go2 issue about: a flag that truncates long slices/arrays e.g.: https://play.golang.org/p/zWL3C7jRlIR

@judepereira
Copy link

@judepereira judepereira commented Dec 31, 2017

@ericlagergren A flag wouldn't do justice to it, unless by default, the String method would return a shorthand representation of the array.

Instead of a flag, maybe we could have String always return a shorthand description of the array, and then a new method, say, Contents which would return the current implementation (i.e. the actual array contents)?

@MironAtHome
Copy link

@MironAtHome MironAtHome commented Jan 1, 2018

something along the line of jar -tvf ?
for example
0 Tue Jul 19 09:36:10 PDT 2011 META-INF/
125 Tue Jul 19 09:36:08 PDT 2011 META-INF/MANIFEST.MF
0 Tue Jul 19 09:36:08 PDT 2011 org/
0 Tue Jul 19 09:36:08 PDT 2011 org/antlr/
0 Tue Jul 19 09:36:08 PDT 2011 org/antlr/runtime/
0 Tue Jul 19 09:36:08 PDT 2011 org/antlr/runtime/misc/
0 Tue Jul 19 09:36:08 PDT 2011 org/antlr/runtime/debug/
0 Tue Jul 19 09:36:08 PDT 2011 org/antlr/runtime/tree/
1211 Tue Jul 19 09:36:08 PDT 2011 org/antlr/runtime/misc/IntArray.class
Any example would help

@judepereira
Copy link

@judepereira judepereira commented Jan 1, 2018

@MironAtHome That could be part of a new API in zip (although I think one exists already - haven't looked).

I was thinking of something on the lines of the file name, and file size.
Contents on the other hand, could return the byte array.

After all, it is the String impl for File, and not zip.

@robpike robpike changed the title archive/zip: *File should implement fmt.Stringer proposal: archive/zip: *File should implement fmt.Stringer Jan 1, 2018
@gopherbot gopherbot added the Proposal label Jan 1, 2018
@robpike robpike removed the NeedsFix label Jan 1, 2018
@dsnet dsnet removed the help wanted label Jan 3, 2018
@rsc
Copy link
Contributor

@rsc rsc commented Jan 8, 2018

Sorry, but you can easily forget and print just about every struct that exists. That doesn't mean every struct should have a String method. It's also unclear whether String should return the name, the content, or (as you suggested) some "ls -l"-like summary. Better not to pick one and let people ask for what they want.

@rsc rsc closed this Jan 8, 2018
@golang golang locked and limited conversation to collaborators Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
10 participants
You can’t perform that action at this time.