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

Allow choosing EOL and appending final newline #1546

Merged
merged 7 commits into from Sep 11, 2021
Merged

Allow choosing EOL and appending final newline #1546

merged 7 commits into from Sep 11, 2021

Conversation

adalinesimonian
Copy link
Contributor

Addresses #951

Adds the following options to js2svg and CLI:

  • eol — can be set to lf or crlf. If unspecified, js2svg uses the platform EOL.
  • finalNewline — defaults to false. If true, js2svg ensures any SVG output has a final newline.

Tests added to cover both options.

Addresses #951

Adds the following options to js2svg and CLI:

- eol — can be set to `lf` or `crlf`. If unspecified, js2svg uses the
  platform EOL.
- finalNewline — defaults to false. If true, js2svg ensures any SVG
  output has a final newline.

Tests added to cover both options.
@TrySound
Copy link
Member

Hi, could you clarify what's the point of finalNewline option?

lib/svgo/js2svg.js Outdated Show resolved Hide resolved
lib/svgo.test.js Outdated Show resolved Hide resolved
@adalinesimonian
Copy link
Contributor Author

Hi, could you clarify what's the point of finalNewline option?

In a number of workspaces, files are required to have a final newline. For example, git diff, GitHub's website, and other tools will either warn of missing newlines or behave improperly when encountering files that are missing a final newline.

lib/svgo.test.js Outdated Show resolved Hide resolved
Aligns test format with refactor made in main branch
lib/svgo/js2svg.js Outdated Show resolved Hide resolved
lib/svgo/js2svg.js Outdated Show resolved Hide resolved
lib/svgo.test.js Outdated Show resolved Hide resolved
EOL is no longer converted to lower case, so only `crlf` and `lf` are
now supported as options, not `CRLF` or `LF` or any other case.
lib/svgo/coa.js Outdated Show resolved Hide resolved
Achieves parity with non-CLI behaviour of not allowing different cases
lib/svgo/coa.js Outdated Show resolved Hide resolved
@TrySound TrySound merged commit e8321f0 into svg:master Sep 11, 2021
@adalinesimonian adalinesimonian deleted the adalinesimonian-eol-config branch September 11, 2021 18:08
@TrySound
Copy link
Member

Thank you for working on this @adalinesimonian!

@adalinesimonian
Copy link
Contributor Author

Thank you for working on this @adalinesimonian!

Наконец-то! It's been a pleasure. 🎉

TrySound added a commit that referenced this pull request Sep 11, 2021
`os` package in js2svg module bothered me for a long time.
We had to hack rollup to mock it for browser.

Thanks to #1546 we now can pass eol from
svgo-node entry point and simplify build.
TrySound added a commit that referenced this pull request Sep 11, 2021
`os` package in js2svg module bothered me for a long time.
We had to hack rollup to mock it for browser.

Thanks to #1546 we now can pass eol from
svgo-node entry point and simplify build.
TrySound added a commit that referenced this pull request Sep 12, 2021
`os` package in js2svg module bothered me for a long time.
We had to hack rollup to mock it for browser.

Thanks to #1546 we now can pass eol from
svgo-node entry point and simplify build.
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.

None yet

2 participants