search

un-ts / prettier

:package: Opinionated but Incredible Prettier plugins.
https://prettier.vercel.app
MIT License
274 stars 25 forks source link

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

Closed frozenbonito closed 2 years ago

frozenbonito commented 2 years ago

close #204

changeset-bot[bot] commented 2 years ago

🦋 Changeset detected

Latest commit: ee030fdc709fdbe2923990132e1f3442a4b1e22f

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[bot] commented 2 years ago

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

🤖 This report was automatically generated by pkg-size-action

codecov[bot] commented 2 years ago

Codecov Report

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

:exclamation: 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%) :arrow_up:

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 commented 2 years ago

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 commented 2 years ago

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: https://github.com/un-ts/prettier/blob/2c1f8c8d9de6702557bdc6273b7ea4f771e5c64e/packages/sql/test/fixtures/basic.sql#L10

After formatting, the final newline have been removed: https://github.com/un-ts/prettier/blob/2c1f8c8d9de6702557bdc6273b7ea4f771e5c64e/packages/sql/test/__snapshots__/fixtures.spec.ts.snap#L29

JounQin commented 2 years ago

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 commented 2 years ago

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

frozenbonito commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

JounQin commented 2 years ago

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

Isn’t prettier core doing this also?

JounQin commented 2 years ago

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 commented 2 years ago

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

JounQin commented 2 years ago

@frozenbonito Thanks for your contribution!

frozenbonito commented 2 years ago

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!