internal/manifest: rework printing and parsing

[nicktrav]

Addresses a number of TODOs by supporting range keys in various printing and parsing methods in manifest.

I opted to split the refactor up into four parts. Each commit is best reviewed in isolation but applies / tests cleanly. This should make reviewing the refactor much easier.

internal/manifest: format bounds consistently

Use the format $filename:[$smallest-$largest] when formatting bounds
for a table. This allows for better code reuse elsewhere.

Update affected test fixtures.

internal/manifest: remove (*Version).Pretty in favor of DebugString

Currently, (*Version).Pretty and (*Version).DebugString are almost
identical, except for the way in which the key is formatted. Remove the
Pretty variant in favor of DebugString, which removes some duplicate
code.

internal/manifest: add file metadata debug string and parser

Add the manifest.(*FileMetadata).DebugString() method that operates in
a similar fashion to manifest.(*Version).DebgString(), printing the
file number and overall bounds for the table. When verbose is set to
true, the point and range key bound are printed, depending on what
type of keys are present in the table (point keys, range keys or both).

Add the manifest.ParseFileMetadataDebug function that operates in a
similar fashion to ParseVersionDebug, parsing the inverse of the debug
string representation of a FileMetadata.

Move the metadata parsing out of ParseVersionDebug and instead rely on
the new function.

Update existing test cases that rely on file metadata parsing to use the
new function.

internal/manifest: consolidate string printing methods

Add a verbose parameter to manifest.(*Version).DebugString that when
set to true prints the verbose representation of each FileMetadata
in each level.

Further consolidate string printing for manifest.Version by having
manifest.(*Version).String use existing code to print the string
representation of a version, with each FileMetadata being printed in
non-verbose mode.

Update existing calls sites in various tests to call String, which is
essentially an alias for DebugString(..., false), which ensures
existing tests do not need to be updated.

[cockroach-teamcity]

This change is 

[jbowens]

Looks great, thanks for doing this!

Reviewed 4 of 4 files at r1, 4 of 4 files at r2, 5 of 5 files at r3, 15 of 15 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @itsbilal and @nicktrav)

---

internal/manifest/version.go, line 268 at r3 (raw file):

	}
	for len(fields) > 0 {
		prefix := strings.TrimSpace(fields[0])

nit: could use return unicode.IsSpace(c) in the FieldsFunc default case to strip whitespace upfront

---

internal/manifest/version.go, line 291 at r3 (raw file):

	// By default, when the parser sees just the overall bounds, we set the point
	// keys. This preserves backwards compatability with existing test cases that
	// specify only the overall bounds.

nice

[nicktrav]

Reviewable status: 8 of 23 files reviewed, 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @nicktrav)

---

internal/manifest/version.go, line 268 at r3 (raw file):

Previously, jbowens (Jackson Owens) wrote…

nit: could use return unicode.IsSpace(c) in the FieldsFunc default case to strip whitespace upfront

Nice. Done.

[nicktrav]

TFTR!