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

js package bug: new line escape character removed incorrectly #424

Closed
JounQin opened this issue Oct 4, 2019 · 7 comments
Closed

js package bug: new line escape character removed incorrectly #424

JounQin opened this issue Oct 4, 2019 · 7 comments

Comments

@JounQin
Copy link

@JounQin JounQin commented Oct 4, 2019

Thanks very much for building this project, and I'm trying to wrap it into a prettier plugin: prettier-plugin-sh, it works well for most cases.

But I just find an issue when formatting Dockerfile:

# input
FROM node:lts-alpine          as       builder

COPY        .      /app
WORKDIR         /app
RUN yarn           --frozen-lockfile &&            \
    yarn       build &&           \
    find . -name      '*.map' -type f -exec rm -f {} \;

FROM          abiosoft/caddy
COPY --from=builder          /app/packages/ufc-host-app/build /srv
EXPOSE      2015
# output
FROM node:lts-alpine as builder

COPY . /app
WORKDIR /app
RUN yarn --frozen-lockfile &&
	yarn build &&
	find . -name '*.map' -type f -exec rm -f {} \;

FROM abiosoft/caddy
COPY --from=builder /app/dist /srv
EXPOSE 2015

It is clear the \ character after && should not be removed.

@kaey

This comment has been minimized.

Copy link
Contributor

@kaey kaey commented Oct 4, 2019

Do you try to run shfmt directly on Dockerfile? This is not supported as Dockerfile is not a shell file its syntax is different and it can break in other ways.

@mvdan

This comment has been minimized.

Copy link
Owner

@mvdan mvdan commented Oct 4, 2019

What @kaey said. Removing the \ is correct in shell, because a line ending in && just continues the command in the following line.

You could potentially work around this by using shfmt -bn, which puts operators like && on the next line, thus needing to add \ before the newline. The printer has the same option. Though I imagine you'd easily run into other issues with Dockerfiles.

@JounQin

This comment has been minimized.

Copy link
Author

@JounQin JounQin commented Oct 5, 2019

Do you try to run shfmt directly on Dockerfile? This is not supported as Dockerfile is not a shell file its syntax is different and it can break in other ways.

Then what's the difference between the JS API and shfmt itself? I've passed the filapath to parser.Parse function:
https://github.com/rx-ts/prettier/blob/master/packages/sh/src/index.ts#L17

@JounQin

This comment has been minimized.

Copy link
Author

@JounQin JounQin commented Oct 5, 2019

@mvdan I'm using the JS API, it there any related API to preserve new line as like as KeepComments: syntax.KeepComments(parser, true)?

@JounQin

This comment has been minimized.

Copy link
Author

@JounQin JounQin commented Oct 6, 2019

@mvdan How can I access functions like BinaryNextLine in the JS API? It seems they are not exported as public API?

Or is the js package just outdated from latest go source?

@mvdan

This comment has been minimized.

Copy link
Owner

@mvdan mvdan commented Oct 8, 2019

@kaey asked you to reproduce the issue with shfmt because that tool ends up running pretty much the same code to format shell code.

The JS package doesn't have printer options like BinaryNextLine right now. It probably will in the future, but again, this won't be enough to support dockerfiles. You're going to need a parser that understands dockerfiles.

@JounQin

This comment has been minimized.

Copy link
Author

@JounQin JounQin commented Oct 11, 2019

@mvdan -bn did the trick, really hope this feature in js package, is there anything I can offer help to achieve this?

Related: foxundermoon/vs-shell-format#31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.