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

Restore newlines in CLI output #42

Closed
wants to merge 1 commit into from
Closed

Conversation

tofumatt
Copy link
Contributor

@tofumatt tofumatt commented Oct 2, 2015

Just realised that when we added the singleLineString template tag in #41, we removed intentional newlines before JSON strings (eg: https://github.com/mozilla/addons-validator/blob/master/src/cli.js#L63).

This restores them in a way I hope isn't offensive. I still think it's nicer (and more explicit/obvious) than just allowing string concat on that line.

r?

@muffinresearch
Copy link
Contributor

Does the newline being removed matter that much? yargs own formatting seems to break up strings anyway. How different is the actual output before and after?

@tofumatt
Copy link
Contributor Author

tofumatt commented Oct 2, 2015

It lets you see the JSON string on one line… but I guess the formatting is a bit weird anyway.

screenshot 2015-10-02 14 53 23

vs:

screenshot 2015-10-02 14 54 21

I added this and meant to ask if it was worth it... and if it was I'd add a test. Now I'm thinking it's silly. Feel free to close if you agree 😉

@muffinresearch
Copy link
Contributor

Yeah we should probably rip the examples out. The cli help doesn't seem like a great place for that. I'll close this for now but if we want it back we can always revisit.

@tofumatt
Copy link
Contributor Author

tofumatt commented Oct 2, 2015

Sounds good. Thanks. 👍

  • tofumatt

On 2 October 2015 at 14:58:31, Stuart Colville (notifications@github.com) wrote:

Yeah we should probably rip the examples out. The cli help doesn't seem like a great place for that. I'll close this for now but if we want it back we can always revisit.


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants