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

Fixes for svgo's stdout mode in 1.0.x #831

Merged
merged 2 commits into from
Nov 7, 2017
Merged

Fixes for svgo's stdout mode in 1.0.x #831

merged 2 commits into from
Nov 7, 2017

Conversation

corpulentcoffee
Copy link
Contributor

1.0.x seems to have introduced some extra junk in the output if you use svgo in stdout mode (i.e. -o -). Using the "with STDIN / STDOUT" usage documented in the README as an example:

$ cat examples/test.svg | bin/svgo -i - -o - > test.min.svg

$ cat test.min.svg 
<svg width="10" height="20">test</svg> 

Done in 10 ms!
0.058 KiB - 35.6% = 0.037 KiB

Obviously that extra stuff doesn't belong in test.min.svg.

This worked correctly in v0.7.x, I believe because the codepaths were more separated and process.stdout.write() was used instead of console.log().

Two proposed changes here:

  • don't call printTimeInfo()/printProfitInfo() unless output != '-'
  • console.log() already prints a trailing newline and it prints spaces between its arguments, so console.log(data, '\n') means <data><space><newline><newline>; we probably just want console.log(data) instead so we get <data><newline>

Thank you for this wonderful project!

When using the `stdout` output mode (e.g. using `svgo -` within a
pipeline), the time and optimization statistics shouldn't be printed in
the output stream.
Not only does `console.log()` already print a newline after its output,
but it prints a space between each of its arguments, so including `'\n'`
as a second argument results in a trailing space, then a newline, then
another newline. Excluding this extra `'\n'` results in just one newline
after the data.
@GreLI GreLI merged commit efcb07e into svg:master Nov 7, 2017
@GreLI
Copy link
Member

GreLI commented Nov 7, 2017

Nice catch!

@corpulentcoffee corpulentcoffee deleted the stdout-mode-fixes branch November 7, 2017 16:04
@GreLI
Copy link
Member

GreLI commented Nov 8, 2017

v1.0.3 with this fix has been just released.

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