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: fmt: improve ergonomics of fmt.Print-like functions #57816

Closed
aarzilli opened this issue Jan 16, 2023 · 15 comments
Closed

proposal: fmt: improve ergonomics of fmt.Print-like functions #57816

aarzilli opened this issue Jan 16, 2023 · 15 comments

Comments

@aarzilli
Copy link
Contributor

aarzilli commented Jan 16, 2023

Proposals #57616, #50554 and #34174 all make a good argument for why functions in the fmt.Print family should be preferred to functions in the fmt.Printf family. However the fmt.Print family is rarely used in practice, so much that people even forget it exists 1 2.

The reasons for fmt.Print neglect are unknown but I propose that the reason is that the fmt.Print family does not have a few frequently used features that can only be accessed through fmt.Printf. I propose we add the following to the fmt package:

// Error formats a string like Sprint and returns it as a value that
// satisfies the error interface.
//
// If exactly one of the argument satisfies the error interface the returned
// error will implemnt an Unwrap method returning it. If there is more than
// one argument satisfying the error interface the Unwrap method will return
// a []error containing all of them.
func Error(a ...any) error

// Type returns the type of a.
func Type(a any) string

// ValueFormatter formats a value like the v verb of Printf
type ValueFormatter struct {
	...
}

// Value returns a ValueFormatter for a.
func Value(a any) *ValueFormatter

// Alt configures the ValueFormatter to print like #v.
func (*ValueFormatter) Alt() *ValueFormatter

// WithFields configures the ValueFormatter to add field names to the output.
func (*ValueFormatter) WithFields() *ValueFormatter

func (*ValueFormatter) String() string

Furthermore I propose that we change the documentation of Sprint to say the following:

Sprint formats using the default formats for its operands and returns the resulting string. Spaces are added between operands when neither is a string or a *ValueFormatter. 

Methodology

I downloaded the 10 most starred Go projects on github according to 3 excluding Go itself and awesome-go (which is a collection of links). I then used grep to find uses of Print and Printf like functions. I found the following uses:

Count Function
78055 Errorf
24355 Sprintf
2218 Println
1622 Printf
1528 Fprintf
1481 Sprint
1077 Debugf
323 Fprint
202 Print

I then parsed the first argument (when it was possible) and identified all formatting directives, including the character immediately following each of them. These were all the formatting directives used at least 100 times:

27305 "%v"
17161 "%d "
12069 "%s"
 8729 "%d"
 5895 "%s "
 4238 "%s:"
 3976 "%q"
 3673 "%v,"
 3401 "%v "
 3202 "%q "
 3170 "%w"
 2924 "%#v"
 1830 "%s,"
 1789 "%+v"
 1563 "%v:"
 1547 "%q:"
 1215 "%s'"
 1129 "%s/"
 1103 "%T"
 1093 "%d:"
 1043 "%q,"
  986 "%s\n"
  963 "%d,"
  944 "%v\n"
  643 "%s."
  607 "%#v,"
  560 "%s]"
  555 "%d]"
  517 "%v'"
  467 "%s-"
  465 "%T "
  369 "%t"
  359 "%d."
  319 "%v]"
  292 "%s%"
  288 "%#v\n"
  261 "%+v "
  249 "%s="
  237 "%+v,"
  233 "%v."
  223 "%#v "
  212 "%d/"
  182 "%x"
  182 "%v/"
  176 "%t,"
  175 "%q."
  165 "%d\n"
  164 "%q\n"
  162 "%d-"
  160 "%v;"
  153 "%#v:"
  143 "%s_"
  131 "%s\""
  119 "%+v\n"
  117 "%s}"
  108 "%s["
  106 "%s;"
  104 "%q]"

Cons

  • fmt.Printf has existed for a long time, it is possible that its use is too entrenched and this new API will never gain any traction. In that case this would add new API surface for no additional benefit.
  • fmt.Print is a worse interface for internationalization compared to fmt.Printf (so are all the string interpolation proposals). We may not want to encourage anything other than fmt.Printf, for this reason.
@gopherbot gopherbot added this to the Proposal milestone Jan 16, 2023
@aarzilli aarzilli changed the title proposal: fmt: add improve ergonomics of fmt.Print-like functions proposal: fmt: improve ergonomics of fmt.Print-like functions Jan 16, 2023
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Jan 16, 2023
@icholy
Copy link

icholy commented Jan 17, 2023

The fmt.Value api doesn't seem like an improvement over fmt.Sprintf("%v", v)

@aarzilli
Copy link
Contributor Author

To me it looks ugly to call fmt.Sprintf from within fmt.Print (or fmt.Sprint). There's precedent for this in the standard library, strconv.FormatInt and strconv.FormatFloat also duplicate the functionality of fmt.Sprintf

@ianlancetaylor
Copy link
Member

I'm not clear on why fmt.Value is needed. What is the difference between fmt.Sprint(x) and fmt.Sprint(fmt.Value(x))?

@ianlancetaylor
Copy link
Member

I don't think it's a good idea for fmt.Error to automatically wrap error values. There are many cases where it is appropriate to use fmt.Errorf with %v for an error value. Wrapping an error is only appropriate when the new error in some sense is an instance of the wrapped error. It's not appropriate when the error merely refers to some other error.

@aarzilli
Copy link
Contributor Author

I'm not clear on why fmt.Value is needed. What is the difference between fmt.Sprint(x) and fmt.Sprint(fmt.Value(x))?

In this case, none, the primary reason is to allow access through fmt.Print to formatting values like %#v, %+v and %#+v do, which at the moment isn't possible without using fmt.Printf. The secondary reason would be to disable automatic insertion of spaces.

I don't think it's a good idea for fmt.Error to automatically wrap error values. There are many cases where it is appropriate to use fmt.Errorf with %v for an error value

In those cases one can call the Error method directly.

@ianlancetaylor
Copy link
Member

In those cases one can call the Error method directly.

To be clear, I think the default should be %v, and %w should be the one that requires special action.

@apparentlymart
Copy link

I do broadly agree that error wrapping is the case that should be explicit, but I wonder if choosing to call fmt.Error instead of fmt.Sprint is a sufficiently-special action to justify the different behavior for this common case.

Perhaps it would be harder to accidentally misuse if renamed and constrained a little more tightly:

  • Name it WrapError instead of just Error.
  • Require the last argument to implement error and disallow any other argument from implementing error.

The second of these is tricky in practice because there's no way to write a variadic signature whose last argument is more constrained than those before it, and so it would need to be declared as WrapError(...any) and impose the type constraints dynamically at runtime. This seems error-prone. I think this would also be the only place in fmt where a value implementing error is forbidden rather than automatically converted to a string using the Error method.

I also considered WrapError(error, ...any) so the error goes first and the prefix components go after, with the result then being the stringified any arguments followed by ": " and then the string representation of the error. But of course that means the arguments are in a different order to how they will appear in the final error string, which seems confusing.


As someone who admitted in the other issue that I don't know why I've been using Sprintf instead of Sprint I do find it plausible that part of the reason is that in practice I'm actually more often using fmt.Errorf, testing.T.Errorf (and similar), and log.Printf than fmt.Sprintf directly. And of those, fmt.Errorf is probably the most common and is therefore encouraging me to use the "f" versions of the others too out of habit.

All of those except fmt.Errorf do have a fmt.Sprint-like equivalent, and so I do quite like the idea of having a fmt.Sprint-like equivalent to fmt.Errorf, but it seems tricky to design in a way that will be self-explanatory to potential users and give good feedback when not used correctly. I'm also unsure about the other proposed features aside from fmt.Error.

Therefore I feel ambivalent about this proposal; I like the idea of it but the details don't feel quite right yet.

@rsc
Copy link
Contributor

rsc commented Mar 15, 2023

I disagree that fmt.Printf should be discouraged over fmt.Print. The helpers suggested here provide essentially the non-f form of Errorf, an implementation of %T (which can also just be reflect.TypeOf), and an implementation of %v. It's unclear why code would call these much longer names instead of writing a printf format. The semantics of Error with respect to Unwrap are also problematic: we used %w as an explicit signal that the error should unwrap. The default %v does not. So Error, if it did exist, should not automatically unwrap to any errors passed in.

@rsc rsc moved this from Incoming to Active in Proposals Mar 15, 2023
@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

@aarzilli
Copy link
Contributor Author

I wrote this because it seemed like there would be interest based on the discussion about string interpolation but it doesn't look like anyone is interested in a simpler syntax for that, besides string interpolation itself.

I'd say everything should be considered retracted besides fmt.Error, which makes sense to me for symmetry.

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

It doesn't seem like there is enough support for fmt.Error (without f), and we probably don't also want fmt.Errorln.

@rsc
Copy link
Contributor

rsc commented Mar 29, 2023

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

@rsc rsc moved this from Active to Likely Decline in Proposals Mar 29, 2023
@aarzilli
Copy link
Contributor Author

Fair enough.

@TheCoreMan
Copy link

I'm not a fan of this proposal itself, but this was a great discussion and a cool methodology to start it off! Thanks @aarzilli

@rsc
Copy link
Contributor

rsc commented Apr 6, 2023

This proposal has been declined as retracted.
— rsc for the proposal review group

@rsc rsc moved this from Likely Decline to Declined in Proposals Apr 6, 2023
@rsc rsc removed this from Proposals Apr 4, 2024
@golang golang locked and limited conversation to collaborators Apr 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants