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: add Formatter fallback #51195

Closed
elagergren-spideroak opened this issue Feb 14, 2022 · 22 comments
Closed

proposal: fmt: add Formatter fallback #51195

elagergren-spideroak opened this issue Feb 14, 2022 · 22 comments
Assignees
Labels
Milestone

Comments

@elagergren-spideroak
Copy link

elagergren-spideroak commented Feb 14, 2022

Implementing fmt.Formatter overrides all other formatting. This makes it difficult to selectively implement verbs: you must either implement every verb and flag combination (including width, etc.) or leave them unimplemented.

For example, I have a UUID type. Its String method defaults to the 36-byte encoding with hyphens. I also want to add support for %x so users can encode it as a 32-byte hexadecimal string. Because UUID implements fmt.Stringer, I have to add %x support via fmt.Formatter (otherwise %x will incorrectly encode the 36-byte encoding as hexadecimal). But this means I lose %q, %X, %v, etc. support. If I want those, I have to implement them myself. See https://go.dev/play/p/lb8mkyhN3-L

Ideally, the fmt package would allow me to write something like

// func Fallback(s State, v rune, arg interface{}) Formatter

func (u UUID) Format(s fmt.State, v rune) {
    switch v {
    case 'x', 'X':
        fmt.Fprintf(s, "%"+string(v), u[:])
    default:
        fmt.Fprintf(s, fmt.Fallback(s, v, u))
    }
}

As another example, I wrote this code (which admittedly I am not very proud of, but hey) to work around the same issue: https://github.com/ericlagergren/decimal/blob/aca2edc11f73e28a0e93d592cc6c3de4a352a81c/big.go#L895

Proposal Template

  • Would you consider yourself a novice, intermediate, or experienced Go programmer?
    Experienced (have been writing Go since 1.3 or so).

  • What other languages do you have experience with?
    Varying degrees of familiarity with C, Java, Python, JavaScript, Bash, Assembly, etc.

  • Would this change make Go easier or harder to learn, and why?
    N/A

  • Has this idea, or one like it, been proposed before?
    Did not immediately find anything.

  • Who does this proposal help, and why?
    Me and other users of the fmt package.

  • What is the proposed change?

Add a mechanism to the fmt package to allow Formatter implementations to "fall back" to the default formatting. For example, an API similar to

// Fallback returns a Formatter that formats arg
// using the default formatting for the State and
// verb.
func Fallback(s State, v rune, arg interface{}) Formatter {
   return fallback{v: arg}
}

type fallback struct {
    v interface{}
}

func (p *pp) handleMethods(verb rune) (handled bool0 {
    [...]
    if f, ok := v.(fallback); ok {
        p.arg = f.arg
    } else if formatter, ok := v.(Formatter); ok {
        [...]
    }
}

The specific API is mostly unimportant to me.

  • Is this change backward compatible?
    Yep.

  • Show example code before and after the change.
    See above.

  • What is the cost of this proposal? (Every language change has a cost).

    • How many tools (such as vet, gopls, gofmt, goimports, etc.) would be affected?
      Very likely none.
    • What is the compile time cost?
      N/A
    • What is the run time cost?
      Depends on how it's implemented.
  • Can you describe a possible implementation?
    See above.

  • How would the language spec change?
    N/A

  • Orthogonality: how does this change interact or overlap with existing features?
    It augments fmt.Formatter. It does not duplicate any existing functionality.

  • Is the goal of this change a performance improvement?
    Nope.

  • Does this affect error handling?
    Nope.

  • Is this about generics?
    Nope.

@ianlancetaylor ianlancetaylor changed the title fmt: add Formatter fallback proposal: fmt: add Formatter fallback Feb 14, 2022
@ianlancetaylor ianlancetaylor added this to the Proposal milestone Feb 14, 2022
@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Feb 14, 2022
@ianlancetaylor
Copy link
Contributor

CC @robpike

@robpike
Copy link
Contributor

robpike commented Feb 15, 2022

Formatter a rarely used interface so I'm not sure if this is a problem widespread enough to need a response. It can be a little annoying to implement, I admit.

I'd call it Default rather than Fallback, but that's a small point. Also it might be cleaner both in implementation and use to provide instead a wrapper type that shields the magic formatting, making it easy recover the default behavior for any verb. That would also provide a solution to the endless issue of String methods accidentally invoking themselves.

I still come back my original point though: Is this a significant enough problem to warrant a general solution?

@elagergren-spideroak
Copy link
Author

elagergren-spideroak commented Feb 15, 2022

@robpike I also prefer Default. I figured Default would return an internal type that handleMethods could sniff out prior to checking for Formatter and such. Perhaps something as simple as type defaultIsAKeyword struct { v interface{} }.

That would also provide a solution to the endless issue of String methods accidentally invoking themselves.

I've been writing Go for over 8 years and still frequently do this :)

I still come back my original point though: Is this a significant enough problem to warrant a general solution?

To answer your question: although fmt.Formatter is not a particularly popular interface, I consistently encounter this frustration when I do use it.

The default verbs and flags are very useful and it's annoying when they aren't implemented. For example, a common use of %q is to delimit the input, so to speak. (*big.Int).Format does not implement %q and on more than one occasion I've forgotten this only to see %!q(big.Int=...) pop up in logs. And so I try to make sure my implementations of Format do the Right Thing.

Here is a random example from our code base. It really only needs to implement two cases: %+v and %v, but ends up using an "interesting" method to handle the other cases. There are a couple other examples where Format implementations are needlessly complex in order to do the Right Thing.

type SomeError []T

func (e SomeError) Format(f fmt.State, c rune) {
	switch {
	case c != 'v':
		fmt.Fprintf(f, makeFormat(f, c), ([]T)(e))
	case !f.Flag('+'):
		fmt.Fprintf(f, makeFormat(f, 's'), e.Error())
	default:
                [...]
	}
}

func makeFormat(f fmt.State, c rune) string {
	var b strings.Builder
	b.WriteByte('%')
	for _, c := range "+-# 0" {
		if f.Flag(int(c)) {
			b.WriteRune(c)
		}
	}
	if width, ok := f.Width(); ok {
		b.WriteString(strconv.Itoa(width))
	}
	if prec, ok := f.Precision(); ok {
		b.WriteByte('.')
		b.WriteString(strconv.Itoa(prec))
	}
	b.WriteRune(c)
	return b.String()
}

@elagergren-spideroak
Copy link
Author

Also, I agree with Go's philosophy of only reluctantly adding new APIs to the standard library, especially to fmt which has had few—if any—API additions. However, I believe that being able to selectively implement verbs (without bending over backward to do so) is core functionality and it's currently missing. I'd love if it were possible to do this without introducing Default.

@robpike
Copy link
Contributor

robpike commented Feb 15, 2022

The original design should perhaps have had a return value to indicate that the default behavior should apply. Too late for that now, although we could add a new interface.

@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

Maybe fmt.DefaultFormat to make clear it's for use by Format? Otherwise, this seems OK.

@rsc
Copy link
Contributor

rsc commented Feb 16, 2022

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 rsc moved this from Incoming to Active in Proposals (old) Feb 16, 2022
@robpike
Copy link
Contributor

robpike commented Feb 16, 2022

I think this one needs a hard think. There are multiple solutions with different complexities, advantages, and tradeoffs.

@robpike
Copy link
Contributor

robpike commented Feb 16, 2022

This is a minor problem in a minor, little-used feature. A simple answer should be sufficient.

A suggestion. In the fmt package, add:

type Default struct {
   Value any
}

When a Default object is printed, all methods on Value are ignored. All that would take is an internal flag in the fmt.pp structure, simple to add.

This would be easy to do and would also provide a clean mechanism for the recursive String method problem.

@elagergren-spideroak
Copy link
Author

@robpike I like the simplicity.

I have a question. What should this program print?

package main

import "fmt"

func main() {
    x := Uint128{1, 2}
    fmt.Printf("%#v\n", x)
}

type Uint128 struct {
    lo, hi uint64
}

func (x Uint128) Format(s fmt.State, v rune) {
    switch v {
    case 's', 'd':
        // print base 10
    case 'x', 'X:
        // print base 16
    default:
        fmt.Fprint(s, fmt.Default{Value: x})
    }
}

Or, more directly: how will the # flag be respected?

@robpike
Copy link
Contributor

robpike commented Feb 16, 2022

I suppose Default could also have an optional State.

As I said above, much thinking is required.

@beoran
Copy link

beoran commented Feb 17, 2022

@robpike I agree this needs a lot of thought. It seems to me the original fmt.Formatter is rather difficult to use. We could probably think of a better design.

@extemporalgenome
Copy link
Contributor

I've also avoided writing custom formatters particularly because there's no great way to override one or two verbs without reimplementing the whole thing, and fmt.State has no trivial way to reconstruct fmt.Formatter inputs into the original verb, in order to pass it, alongside a conversion of the receiver to a derived type, to fmt.Fprintf.

Perhaps we could just document that when fmt.State receives no writes, it'll just re-interpret the value as though it were not a fmt.Formatter, and proceed accordingly (thus the "fallback" would merely be returning from the Format method without the fmt.State.Write method having been called).

@kevinburke
Copy link
Contributor

We could also implement e.g. golang.org/x/tools/fmt/fmtutil.Fallback, write about it, measure adoption, and then make a decision from there.

@ianlancetaylor
Copy link
Contributor

See #51668.

@robpike robpike self-assigned this Mar 22, 2022
@robpike
Copy link
Contributor

robpike commented Apr 19, 2022

Putting this on hold until whether we see whether the solution to #51668 (which is suggested implicitly in the original comment here, one is surprised to see) solves the problem well enough. It well might.

@rsc rsc moved this from Active to Hold in Proposals (old) May 4, 2022
@rsc
Copy link
Contributor

rsc commented May 4, 2022

Placed on hold.
— rsc for the proposal review group

@gopherbot
Copy link

Change https://go.dev/cl/400875 mentions this issue: fmt: add a function to recover the original format string given a State

gopherbot pushed a commit that referenced this issue Aug 6, 2022
Sometimes when implementing a Formatter it's helpful to use the fmt
package without invoking the formatter. This new function, FormatString,
makes that easier in some cases by recreating the original formatting
directive (such as "%3.2f") that caused Formatter.Format to be
called.

The original Formatter interface is probably not what we would
design today, but we're stuck with it. FormatString, although it
takes a State as an argument, compensates by making Formatter a
little more flexible.

The State does not include the verb so (unlike in the issue), we
must provide it explicitly in the call to FormatString. Doing it there
minimizes allocations by returning the complete format string.

Fixes #51668
Updates #51195

Change-Id: Ie31c8256515864b2f460df45fbd231286b8b7a28
Reviewed-on: https://go-review.googlesource.com/c/go/+/400875
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
jproberts pushed a commit to jproberts/go that referenced this issue Aug 10, 2022
Sometimes when implementing a Formatter it's helpful to use the fmt
package without invoking the formatter. This new function, FormatString,
makes that easier in some cases by recreating the original formatting
directive (such as "%3.2f") that caused Formatter.Format to be
called.

The original Formatter interface is probably not what we would
design today, but we're stuck with it. FormatString, although it
takes a State as an argument, compensates by making Formatter a
little more flexible.

The State does not include the verb so (unlike in the issue), we
must provide it explicitly in the call to FormatString. Doing it there
minimizes allocations by returning the complete format string.

Fixes golang#51668
Updates golang#51195

Change-Id: Ie31c8256515864b2f460df45fbd231286b8b7a28
Reviewed-on: https://go-review.googlesource.com/c/go/+/400875
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Russ Cox <rsc@golang.org>
@dnesting
Copy link

Just to follow up, since I found this searching for a solution to the same problem, the addition of fmt.FormatString does seem to fairly reliably solve this problem for me, eg:

func (t T) Format(f fmt.State, c rune) {
    switch c {
    case 'd':
        fmt.Fprint(f, "Ceci n'est pas un nombre")
    case 's':
        fmt.Fprint(f, t.String())
    default:
        type hideMethods T
        type T hideMethods
        fmt.Fprintf(f, fmt.FormatString(f, c), T(t))
    }
}

https://go.dev/play/p/BkqOaS4CL68

This can be simplified further if the underlying type of T isn't a struct and you're OK losing the type name in %#v.

@robpike
Copy link
Contributor

robpike commented May 25, 2023

Thanks for the report, @dnesting. Maybe this can be closed now.

@rsc
Copy link
Contributor

rsc commented Jun 7, 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 rsc removed the Proposal-Hold label Jun 7, 2023
@rsc
Copy link
Contributor

rsc commented Jun 14, 2023

No change in consensus, so declined.
— rsc for the proposal review group

@rsc rsc closed this as completed Jun 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Declined
Development

No branches or pull requests

10 participants