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

JSONL output and interface refactoring #148

Merged
merged 8 commits into from
Aug 9, 2016
Merged

Conversation

BrianHicks
Copy link
Contributor

This PR adds JSONL support to the prettyprinter. This was really just an excuse to get to know the pretty printer better for another feature, and in the process addresses #146 and #147.

@rebeccaskinner can you review this, especially 876c581? If you feel strongly that this is the wrong direction for that interface, we can remove the commit.

Accurately, split it out into two parts. In the base we don't need to
know if a string is visible, so `Renderable` is now just a wrapper over
`fmt.Stringer`. I've changed the implementation of `String` on
`stringRenderable` to return the proper value if the string is
hidden (an empty string.) Because of this, `Render` is gone as well.

VisibleRenderable takes on the simplifications mentioned in #46. Namely,
it removes `Hide` and `Unhide`. These weren't actually used outside the
internal renderable implementation, so they didn't need to be part of
the interface.

There is some cleanup that had to take place to make this new split
valid, but not too much. Mostly removing utility methods in favor of
just calling `String` and/or wrapping renderables. In basically all
cases, we can just unwrap the string at the call site instead of using a
utility. This may lose a little fidelity, in which case the caller
should definitely be requiring a `VisibleRenderable` instead of just
`Renderable`.

`SprintfVisible` was only needed in one case, it may yet be removed.

Fixes #146
@rebeccaskinner rebeccaskinner merged commit 369a07f into master Aug 9, 2016
@BrianHicks BrianHicks deleted the feature/nice-output branch October 19, 2016 18:40
BrianHicks pushed a commit that referenced this pull request Dec 22, 2016
JSONL output and interface refactoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants