Skip to content

Invalid indentaion #21

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

Open
zdm opened this issue Apr 10, 2025 · 25 comments
Open

Invalid indentaion #21

zdm opened this issue Apr 10, 2025 · 25 comments

Comments

@zdm
Copy link

zdm commented Apr 10, 2025

Hi,

Multiline command indentation is broken.

Result:

RUN \
 # install dependencies
 NODE_ENV=production npm install-clean \
 # cleanup
 && /usr/bin/env bash <(curl -fsSL https://raw.githubusercontent.com/softvisio/scripts/main/env-build-node.sh) cleanup

I have 4 spaces indentation configured, but it always make 1 space indent, which is not very readable.

Link to the original issue: un-ts/prettier#441 (comment)

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

What is your dockerfmt version and invocation?

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

This report is not consistent with my test suite output from 0.3.4. https://github.com/reteps/dockerfmt/blob/main/tests/out/run5.dockerfile

@zdm
Copy link
Author

zdm commented Apr 10, 2025

dockerfmt: v0.3.2

@zdm
Copy link
Author

zdm commented Apr 10, 2025

Invoked from the prettier-plugin-sh, I don't know details.

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

You need dockerfmt 0.3.4. It should be published.

@reteps reteps closed this as completed Apr 10, 2025
@zdm
Copy link
Author

zdm commented Apr 10, 2025

Not published yet, will wait.

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

What leads you to believe it isn't published? You should double check whatever you are claiming before you write it...

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

@zdm
Copy link
Author

zdm commented Apr 10, 2025

npm up doesn't install latest version

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

I cannot reproduce. I made an empty repo, and did npm i prettier-plugin-sh. It used 0.3.4.

@zdm
Copy link
Author

zdm commented Apr 10, 2025

With v0.3.4 result is the same, indented with 1 space, ingoting settings.

@reteps
Copy link
Owner

reteps commented Apr 10, 2025

If you want to report a bug in the JS formatter wrapper, please report it there. If you want to report a bug in the golang parser, please show your dockerfmt version and invocation. That is, invoke the go binary on the command line.

@JounQin
Copy link
Contributor

JounQin commented Apr 11, 2025

@reteps

@reteps/dockerfmt v0.3.4

import { formatDockerfileContents } from '@reteps/dockerfmt'

console.log(
  await formatDockerfileContents(
    `
RUN \
    # install dependencies
    NODE_ENV=production npm install-clean \
    \
    # cleanup
    && /usr/bin/env bash <(curl -fsSL https://raw.githubusercontent.com/softvisio/scripts/main/env-build-node.sh) cleanup
`.trim(),
    {
      indent: 4,
      spaceRedirects: false,
      trailingNewline: true,
    },
  ),
)

Output:

RUN \
 # install dependencies

@reteps
Copy link
Owner

reteps commented Apr 11, 2025

@reteps

@reteps/dockerfmt v0.3.4

import { formatDockerfileContents } from '@reteps/dockerfmt'

console.log(
await formatDockerfileContents(
RUN \ # install dependencies NODE_ENV=production npm install-clean \ \ # cleanup && /usr/bin/env bash <(curl -fsSL https://raw.githubusercontent.com/softvisio/scripts/main/env-build-node.sh) cleanup.trim(),
{
indent: 4,
spaceRedirects: false,
trailingNewline: true,
},
),
)
Output:

RUN \

install dependencies

I can't replicate this -- can you show the bug as replicated on the CLI?

@zdm
Copy link
Author

zdm commented Apr 11, 2025

In 0.3.5 bug still exists.
The code above reproduces the issue.

@reteps
Copy link
Owner

reteps commented Apr 11, 2025

If you want to report a bug in the JS formatter wrapper, please report it there. If you want to report a bug in the golang parser, please show your dockerfmt version and invocation. That is, invoke the go binary on the command line.

@JounQin
Copy link
Contributor

JounQin commented Apr 11, 2025

I can't replicate this -- can you show the bug as replicated on the CLI?

@reteps What do you mean? I'm using the js API, not CLI? Isn't it the WASM/js library you provided?

@reteps
Copy link
Owner

reteps commented Apr 11, 2025

I can't replicate this -- can you show the bug as replicated on the CLI?

What do you mean? I'm using the js API, not CLI?

It's hard for me to tell if this is a bug in the JS API or in the internal golang formatter. If I can replicate through the CLI, I can fix the issue much easier.

@reteps
Copy link
Owner

reteps commented Apr 11, 2025

If there is a difference in behavior between the WASM library and the golang library, then potentially there is an issue in the WASM bindings.

@reteps reteps reopened this Apr 11, 2025
@JounQin
Copy link
Contributor

JounQin commented Apr 11, 2025

Then it's an issue from the js/wasm API? For example, byte size, etc.

@reteps
Copy link
Owner

reteps commented Apr 11, 2025

If you can't replicate it via the CLI, then it must be an issue with the wasm API.

@JounQin
Copy link
Contributor

JounQin commented Apr 17, 2025

If you can't replicate it via the CLI, then it must be an issue with the wasm API.

@reteps Yes, then are we going to resolve the WASM API issue here?

@zdm
Copy link
Author

zdm commented Apr 26, 2025

Possible to fix this? Does any help needed?

@reteps
Copy link
Owner

reteps commented Apr 26, 2025

Help is wanted; this seems related to #25 as well. They are both JS WASM issues that I don't have a clear solution for.

@zdm
Copy link
Author

zdm commented Apr 26, 2025

I don't understand anything in go. ((
Can help with js and testing.

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

No branches or pull requests

3 participants