feat(sql)!: respect endOfLine option from prettier core

[frozenbonito]

close #204

[changeset-bot]

🦋 Changeset detected

Latest commit: ee030fd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
prettier-plugin-sql	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[github-actions]

📊 Package size report   No changes

File	Before	After
Total (Includes all files)	211.6 kB	211.6 kB
Tarball size	68.1 kB	68.1 kB

Unchanged files

File	Size
.changeset/config.json	313 B
.changeset/happy-lobsters-shop.md	65 B
.changeset/README.md	510 B
.codesandbox/ci.json	38 B
.editorconfig	161 B
.eslintrc	174 B
.github/FUNDING.yml	195 B
.github/workflows/ci.yml	757 B
.github/workflows/pkg-size.yml	496 B
.github/workflows/release.yml	1.1 kB
.github/workflows/vercel.yml	814 B
.lintstagedrc.cjs	50 B
.nvmrc	6 B
.postcssrc.cjs	51 B
.prettierignore	22 B
.prettierrc	24 B
.remarkrc	60 B
.simple-git-hooks.cjs	51 B
.stylelintignore	124 B
.stylelintrc	42 B
assets/pkg.svg	15.1 kB
assets/sh.png	14.4 kB
CHANGELOG.md	324 B
docs/App.tsx	1.4 kB
docs/global.scss	156 B
docs/index.tsx	208 B
docs/package.json	112 B
docs/tsconfig.json	85 B
LICENSE	1.1 kB
package.json	3.3 kB
packages/pkg/CHANGELOG.md	9.5 kB
packages/pkg/LICENSE	16.7 kB
packages/pkg/package.json	1.7 kB
packages/pkg/README.md	6.3 kB
packages/pkg/src/index.ts	1.4 kB
packages/pkg/src/rules/files.ts	1.6 kB
packages/pkg/src/rules/object.ts	736 B
packages/pkg/src/rules/sort.ts	2.3 kB
packages/pkg/src/types.ts	996 B
packages/pkg/src/utils.ts	502 B
packages/pkg/test/__snapshots__/engines.spec.ts.snap	159 B
packages/pkg/test/__snapshots__/files.spec.ts.snap	174 B
packages/pkg/test/__snapshots__/test.spec.ts.snap	2.4 kB
packages/pkg/test/engines.spec.ts	437 B
packages/pkg/test/files.spec.ts	429 B
packages/pkg/test/fixtures/fixture1.json	1.0 kB
packages/pkg/test/fixtures/fixture2.json	863 B
packages/pkg/test/test.spec.ts	2.1 kB
packages/pkg/tsconfig.json	154 B
packages/sh/CHANGELOG.md	9.9 kB
packages/sh/LICENSE	1.1 kB
packages/sh/package.json	1.9 kB
packages/sh/README.md	4.7 kB
packages/sh/src/index.ts	6.3 kB
packages/sh/test/__snapshots__/fixture.spec.ts.snap	27.4 kB
packages/sh/test/__snapshots__/shellscript.spec.ts.snap	168 B
packages/sh/test/fixture.spec.ts	1.1 kB
packages/sh/test/fixtures/.dockerignore	108 B
packages/sh/test/fixtures/.properties	177 B
packages/sh/test/fixtures/133.sh	5.2 kB
packages/sh/test/fixtures/146.ini	1.1 kB
packages/sh/test/fixtures/147.cfg	3.3 kB
packages/sh/test/fixtures/148.ini	3.1 kB
packages/sh/test/fixtures/162.sh	15.7 kB
packages/sh/test/fixtures/182	1.9 kB
packages/sh/test/fixtures/Dockerfile	394 B
packages/sh/test/fixtures/hosts	406 B
packages/sh/test/fixtures/jvm.options	162 B
packages/sh/test/fixtures/no-ext	37 B
packages/sh/test/fixtures/shell.sh	368 B
packages/sh/test/shellscript.spec.ts	478 B
packages/sh/tsconfig.json	154 B
packages/sql/CHANGELOG.md	4.0 kB
packages/sql/LICENSE	1.1 kB
packages/sql/package.json	1.9 kB
packages/sql/README.md	5.4 kB
packages/sql/src/index.ts	10.4 kB
packages/sql/test/__snapshots__/fixtures.spec.ts.snap	425 B
packages/sql/test/__snapshots__/sql.spec.ts.snap	346 B
packages/sql/test/fixtures.spec.ts	1.0 kB
packages/sql/test/fixtures/144.sql	68 B
packages/sql/test/fixtures/basic.sql	120 B
packages/sql/test/sql.spec.ts	681 B
packages/sql/tsconfig.json	154 B
README.md	6.0 kB
scripts/format.ts	682 B
scripts/languages.ts	3.3 kB
tsconfig.base.json	86 B
tsconfig.json	182 B
vercel.json	325 B

_{🤖 This report was automatically generated by pkg-size-action}

[codecov]

Codecov Report

Merging #207 (8c2a3ba) into master (2c1f8c8) will increase coverage by 0.58%.
The diff coverage is 100.00%.

❗ Current head 8c2a3ba differs from pull request most recent head ee030fd. Consider uploading reports for the commit ee030fd to get more accurate results

@@            Coverage Diff             @@
##           master     #207      +/-   ##
==========================================
+ Coverage   83.96%   84.54%   +0.58%     
==========================================
  Files           7        7              
  Lines         106      110       +4     
  Branches       27       28       +1     
==========================================
+ Hits           89       93       +4     
  Misses         17       17              

Impacted Files	Coverage Δ
packages/sql/src/index.ts	65.38% <100.00%> (+6.29%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2c1f8c8...ee030fd. Read the comment docs.

[JounQin]

Thanks for this PR!

But should we reuse the endOfLine option from prettier core?

https://prettier.io/docs/en/options.html#end-of-line

The previous behavior could be a new option value avoid as the default value for compatibility.

[frozenbonito]

Thanks for comment.

But should we reuse the endOfLine option from prettier core?

sql-formatter seems to support only lf.

https://github.com/sql-formatter-org/sql-formatter/blob/c59a163cfc93d9780480f4c262f9efa6fd1038ad/src/formatter/Layout.ts#L85-L86

It means the end of other each line is always lf, so I think the final line should be so too.

The previous behavior could be a new option value avoid as the default value for compatibility.

In previous version, it always removes a final newline, doesn't preserve original.

There is a final newline here:

prettier/packages/sql/test/fixtures/basic.sql

Line 10 in 2c1f8c8

UPDATE a SET id = 1 WHERE name IN (SELECT name FROM b)

After formatting, the final newline have been removed:

prettier/packages/sql/test/__snapshots__/fixtures.spec.ts.snap

Line 29 in 2c1f8c8

)"

[JounQin]

sql-formatter seems to support only lf.

We can adapt this in prettier-plugin-sql by simple formatted.replace(/\r?\n/g, ending)

In previous version, it always removes a final newline, doesn't preserve original.

That's why I say we should use `avoid` as the default value.

endOfLine is for all lines, so avoid should not be presented, maybe we should just keep the previous behavior on auto.

This would a BREAKING CHANGE as prettier use lf by default now, but this feature is on v0.10.0, so breaking is allowed.

[frozenbonito]

I added a new commit. Do you mean like this?

[frozenbonito]

Hmm, I think it is a strange that endOfLine affects final newline.

In addition, if this plugin respects endOfLine, I think it should preserve original line ending on auto same as prettier.

"auto" - Maintain existing line endings (mixed values within one file are normalised by looking at what’s used after the first line)

Acutually, however, even if the original line ending is crlf, it will be replaced with lf by sql-formatter.
This is an unexpected behavior for users.

Sorry, it's my misunderstanding.

[frozenbonito]

All crlf will replaced with lf by sql-formatter, but prettier core replaces them with a value based endOfLine again.
The same is true for the final newline, formatted + '\n' works as expected without reference to endOfLine.

[frozenbonito]

For that reason, I think we just need an option to control the final newline for this plugin.

[JounQin]

Hmm, I think it is a strange that endOfLine affects final newline.

Isn’t prettier core doing this also?

[JounQin]

It seems there is no way to get original auto endOfLine value

https://github.com/prettier/prettier/blob/ab72a2c11c806f3a8a5ef42314e291843e1b3e68/src/common/end-of-line.js#L3-L9

So a new option seems really needed, thanks!

[JounQin]

I'm thinking is there any user case to have no final new line support? Prettier always print final new line AFAIK.

[JounQin]

@frozenbonito Thanks for your contribution!

[frozenbonito]

I'm thinking is there any user case to have no final new line support? Prettier always print final new line AFAIK.

I agree with you.

Thank you for refactoring and merging!