(More) Reproducible output

[hoijui]

without the here present "Filters out blank lines before outputting" commit,
repeated application of the filter on an SVG adds more and more blank lines.
With this commit, all blank lines get purged, resulting in the same output (in my case), whether applying gsvg only once or many times to a single SVG.

The other three commits are just cosmetics.

Thank you for this project! :-)

[noahbrenner]

Thanks for the PR, @hoijui! Sorry it took me so long to review it -- I didn't have PR notifications turned on and I missed it.

You have a few separate types of changes here. In general, it's good to make separate PRs so that discussion about each one can have a narrower focus, but don't worry about that this go round!

.gitignore

Great, no objections!

Making cli.js executable

Yes, good catch!

Removing blank lines

This could be a useful feature to add, but it'll take a little more work than it seems on the surface:

• Since this changes GSVG's functionality, it needs to come with test cases.

• This functionality has the potential to change the rendering of an SVG file in certain circumstances, so it needs to be optional and disabled by default.

  How could it break something? Depending on the SVG version, you can use one of the following to preserve whitespace when rendering, instead of collapsing down to 1 space. In a file that uses such functionality on an element with blank lines, removing those lines would change the rendered result:

  <text style="white-space: pre">foo
  
  bar</text>
  <text xml:space="preserve">baz
  
  buzz</text>

If you're up for diving back in to get this feature merged, here are the steps needed:

• lib/renderer.js
  • The exported function accepts an options object as a parameter. It should accept a boolean options.removeBlankLines property to control the new behavior.
  • The logic you wrote should go in this file instead of index.js. The actual removal of blank lines should happen right before the return statement, inside an if block where you'll reassign linesOut after filtering.
  • Also see the comments below on the index.js file. You'll be moving that code, but you can make the changes when moving it.
• Documentation
  • Update the help text in cli.js, adding an entry in the Flags section. Let's call the option -b/--remove-blank-lines.
  • in README.md, make the same change as above in the CLI > More Detailed section.
  • in README.md, document the new API option (removeBlankLines) in the API > Arguments (for either function) > options section.
  • Optional: add an example in the API > Examples section of README.md.
  • Optional: mention in both the CLI and API sections that this option can potentially cause problems in SVG files that use style="white-space: pre" or xml:space="preserve".
• Tests (When you're working on tests, you might want to run npx ava --watch in your terminal so that tests are re-run any time you make a change)
  • test/cli.test.js should contain tests that verify whether options are parsed correctly
    • -b in the section labeled // short flags are translated to their long counterparts
    • --remove-blank-lines in the section labeled // flags are parsed correctly, testing both with and without the flag.
  • test/module.test.js should contain tests the resulting output
    • Make sure to test the behavior with removeBlankLines set to true, false, and omitted.

Feel free to ask questions as you work on this. I'll be happy to help!

[noahbrenner]

It's generally considered bad practice to modify prototypes of builtins. This could just be a standalone named function. However, since the function is so simple, let's just put this logic inline inside removeBlankLines() (which already has a descriptive enough name to clearly explain what it's doing).

[noahbrenner]

For consistency with the rest of the codebase, let's get rid if the space after any !s.

See my main comment for where this function should go.