un-ts / prettier

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

[prettier-plugin-sh] Incorrectly splits RUN command #339

Closed dbrugger closed 5 months ago

dbrugger commented 5 months ago

Repro: https://github.com/dbrugger/prettier/blob/Dockerfile-cmd-split-repro/packages/sh/test/fixtures/run-semicolon.Dockerfile

Input:

RUN echo hi; echo hello

Output:

RUN echo hi
echo hello

Expected: keep as is

JounQin commented 5 months ago

Can you add \ for workaround? The support Dockerfile is very basic.

RUN echo hi && \
echo hello
dbrugger commented 5 months ago

As workaround I added Dockerfile to .prettierignore. The case here is not very complex, and very standard. If the plugin does not intend to support such, would it not be better to disable that function by default in the plugin?

dbrugger commented 5 months ago

An important distinction also is:

  1. Not supporting pretty formatting
  2. Breaking the user's code

In this case it is (2).

JounQin commented 5 months ago

If the plugin does not intend to support such, would it not be better to disable that function by default in the plugin?

I'm not certain actually, many users are just writing simple Dockerfile files as I described and they (including myself) want it can be enabled by default.

But it could be a good chance to add a related section describing the limitation.

Not supporting pretty formatting

I'm not for sure to understand this.

FelixZY commented 2 months ago

@JounQin I had this issue in a dockerfile today:

FROM debian:bookworm
ENV ASPNETCORE_URLS http://+:5000;https://+:5001

is formatted as

FROM debian:bookworm
ENV ASPNETCORE_URLS http://+:5000
https://+:5001

I do not think I can add \ as a workaround for this case.


EDIT

I was able to work around this using quotes around the env value. However, I still think this is a bug worth addressing. It might also be good to support some type of range ignore (see https://prettier.io/docs/en/ignore.html#range-ignore).