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

fmt: State could be a little friendlier #25150

Open
seebs opened this issue Apr 28, 2018 · 5 comments
Open

fmt: State could be a little friendlier #25150

seebs opened this issue Apr 28, 2018 · 5 comments
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@seebs
Copy link
Contributor

seebs commented Apr 28, 2018

What version of Go are you using (go version)?

1.10

Does this issue reproduce with the latest release?

yes.

What operating system and processor architecture are you using (go env)?

linux/amd64, but seems irrelevant

What did you do?

Tried to use Formatter efficiently.

https://play.golang.org/p/sHI8UPe_f98

What did you expect to see?

I don't honestly know. I was hoping for a reasonably straightforward way to express "use this format string for several things that I wish to format".

What did you see instead?

See playground example.

In this specific use case, a fair bit of what I want could be accomplished much more easily if fmt.State had a member Format []bytes such that fmt.Sprintf(s.Format, value) would replicate the format specifier that created the fmt.State. (But with, for instance, a * width or precision already baked in.)

It also almost feels like fmt.State arguments could be used with Printf, etc., as a sort of meta-specifier. Consider a hypothetical %S format specifier, which consumes two arguments, the first being a fmt.State and the second being a value. So, once you have a format.State which specifies a given width/precision/flags, you could do something like:

fmt.Fprintf(s, "[%S, %S, %S]", s, x0, s, x1, s, x2)

(There's an interesting line of inquiry with casting the members to a type that has a custom formatting function, but because you can't nest functions, you can't declare the type and its formatter inside the Format() function for the containing type, and if the formatter is outside it, you can't have different goroutines simultaneously printing objects using different formats.)

(It might also be useful to mention explicitly in the fmt docs that Formatter can be used with arbitrary runes, as long as they're not %, T, or v.)

@ALTree ALTree changed the title pkg/fmt: fmt.State could be a little friendlier fmt: fmt.State could be a little friendlier Apr 29, 2018
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 29, 2018
@odeke-em odeke-em changed the title fmt: fmt.State could be a little friendlier fmt: State could be a little friendlier Apr 30, 2018
@odeke-em
Copy link
Member

/cc @robpike

@bcmills
Copy link
Contributor

bcmills commented Apr 30, 2018

It seems like you could already factor out a function to do most of what you want independent of the standard library:

func FmtString(f fmt.State) string

Then you could use it like:

fs := FmtString(s)
fmt.Fprintf(s, fmt.Sprintf("[%s, %s, %s]", fs, fs, fs), x0, x1, x2)

That's admittedly not quite as efficient as what you're describing, but you can write it today to at least prove out the API (and look for call sites that would benefit from it).

If that turns out to be broadly useful, you could imagine folding that back into the standard library without expanding the exported API at all: if we added a String method to the implementation of fmt.State, then FmtString(s) could just be fmt.Sprint(s).

@kortschak
Copy link
Contributor

I've bumped into this on a few occasions. fmt.State does not make it easy or safe at all to access the original construction of the state. At the moment I am trying to mimic complex128 printing for quaternions, this should be easy, but is not because there are many flags and verbs that interact with floats. The fmt.State that the Format call gets knows all this, but has to be pulled out piecemeal and with reference to the fmt implementation for float printing (rather than just with reference to the documentation).

@sfllaw
Copy link
Contributor

sfllaw commented Apr 24, 2020

Might I suggest fmt.Fmt that can be used alongside fmt.pp.String?

// Fmt returns the formatting verb and flags represented by f and c.
// Fmt is typically used by Formatter.Format implementations to implement Printf.
func Fmt(f fmt.State, c rune) string {
	var s string
	switch f := f.(type) {
	case fmt.Stringer:
		s = f.String()
	default:
		s = fmt.Sprint(f)
	}
	return "%" + s + string(c)
}

// SF is a type that formats the formatting verb.
type SF struct {}

func (sf SF) Format(f State, c rune) {
	Fprint(f, Fmt(f, c))
}

func TestFmt(t *testing.T) {
	for i, tt := range fmtTests {
		t.Run(strconv.Itoa(i), func(t *testing.T) {
			// %p and %T verbs do not call tt.val.Format.
			if strings.HasSuffix(tt.fmt, "p") {
				t.Skipf("%q: %%p verb does not call Format", tt.fmt)
			} else if strings.HasSuffix(tt.fmt, "T") {
				t.Skipf("%q: %%T verb does not call Format", tt.fmt)
			}

			if strings.Contains(tt.out, "%!") {
				t.Skipf("Sprintf(%q, %#v) results in format error %q", tt.fmt, tt.val, tt.out)
			}

			format := Sprintf(tt.fmt, SF{})
			s := Sprintf(format, tt.val)
			i := strings.Index(tt.out, "PTR")
			if i >= 0 && i < len(s) {
				var pattern, chars string
				switch {
				case strings.HasPrefix(tt.out[i:], "PTR_b"):
					pattern = "PTR_b"
					chars = "01"
				case strings.HasPrefix(tt.out[i:], "PTR_o"):
					pattern = "PTR_o"
					chars = "01234567"
				case strings.HasPrefix(tt.out[i:], "PTR_d"):
					pattern = "PTR_d"
					chars = "0123456789"
				case strings.HasPrefix(tt.out[i:], "PTR_x"):
					pattern = "PTR_x"
					chars = "0123456789abcdef"
				case strings.HasPrefix(tt.out[i:], "PTR_X"):
					pattern = "PTR_X"
					chars = "0123456789ABCDEF"
				default:
					pattern = "PTR"
					chars = "0123456789abcdefABCDEF"
				}
				p := s[:i] + pattern
				for j := i; j < len(s); j++ {
					if !strings.ContainsRune(chars, rune(s[j])) {
						p += s[j:]
						break
					}
				}
				s = p
			}
			if s != tt.out {
				t.Errorf("%q: Sprintf(%q, %#v) = %q expected %q", tt.fmt, format, tt.val, s, tt.out)
			}
		})
	}
}

@kevinburke
Copy link
Contributor

I agree with the points made in the discussion here, and pulled one piece out into a formal proposal: #51668

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

8 participants