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

deno fmt double format issue with single line for stmt with call expr with arrow function argument #12686

Closed
KnorpelSenf opened this issue Nov 8, 2021 · 4 comments
Assignees
Labels
bug Something isn't working correctly deno fmt Related to the "deno fmt" subcommand or dprint

Comments

@KnorpelSenf
Copy link
Contributor

I would expect that for any code base the command deno fmt && deno fmt --check succeeds. However, this is not the case, as the following example illustrates.

$ cat file.ts # badly formatted file
const buttonCount = 10;

function foo() {
  for (let i = 0; i < buttonCount; i++) { range.text(i.toString(), (ctx) => ctx.reply(`${i} selected`));
  }
}
$ deno fmt --check file.ts

from /tmp/file.ts:
4 | -  for (let i = 0; i < buttonCount; i++) { range.text(i.toString(), (ctx) => ctx.reply(`${i} selected`));
4 | +  for (let i = 0; i < buttonCount; i++) {
5 | +    range.text(i.toString(), (ctx) =>
6 | +      ctx.reply(`${i} selected`));
7 | -
8 | -
9 | +

error: Found 1 not formatted file in 1 file
$ deno fmt file.ts 
/tmp/file.ts
Checked 1 file
$ deno fmt file.ts --check

from /tmp/fie.ts:
5 | -    range.text(i.toString(), (ctx) =>
6 | -      ctx.reply(`${i} selected`));
5 | +    range.text(i.toString(), (ctx) => ctx.reply(`${i} selected`));

error: Found 1 not formatted file in 1 file
$ deno fmt file.ts 
/tmp/file.ts
Checked 1 file
$ deno fmt file.ts --check
Checked 1 file
@kitsonk kitsonk added bug Something isn't working correctly deno fmt Related to the "deno fmt" subcommand or dprint labels Nov 8, 2021
@dsherret dsherret changed the title deno fmt is not idempotent deno fmt double format issue with single line for stmt with call expr with arrow function argument Nov 8, 2021
@ghost
Copy link

ghost commented Nov 27, 2022

@dsherret @KnorpelSenf

When troubleshooting the following using the order of command, I do not receive the same errors that you are receiving. Seems to be working fine for me. I am running Windows 11 and executing via Powershell running the trunk version as it currently stands today. You are saying that you entered the commands in this order and had to run deno fmt twice?

cat file.ts
deno fmt --check file.ts
deno fmt file.ts
deno fmt --check file.ts
cat file.ts
const buttonCount = 10;

function foo() {
  for (let i = 0; i < buttonCount; i++) { range.text(i.toString(), (ctx) => ctx.reply(`${i} selected`));
  }
}

cargo run -- fmt --check .\file.ts
4 | -  for (let i = 0; i < buttonCount; i++) { range.text(i.toString(), (ctx) => ctx.reply(`${i} selected`));
4 | +  for (let i = 0; i < buttonCount; i++) {
5 | +    range.text(i.toString(), (ctx) =>
6 | +      ctx.reply(`${i} selected`));
6 | -}
8 | +}
9 | +

PS C:\Code\Deno\deno> cargo run -- fmt .\file.ts
    Finished dev [unoptimized + debuginfo] target(s) in 0.72s
     Running `target\debug\deno.exe fmt .\file.ts`
C:\Code\Deno\deno\file.ts
Checked 1 file

PS C:\Code\Deno\deno> cargo run -- fmt --check .\file.ts
    Finished dev [unoptimized + debuginfo] target(s) in 0.72s
     Running `target\debug\deno.exe fmt --check .\file.ts`
Checked 1 file

@0f-0b
Copy link
Contributor

0f-0b commented Nov 27, 2022

These commands work now because #14314 changed deno fmt to retry until the formatting is stable. The underlying issue remains unresolved and can still be observed when formatting the file in an editor with deno lsp.

@KnorpelSenf
Copy link
Contributor Author

KnorpelSenf commented May 29, 2024

@0f-0b does that actually mean that deno fmt does one useless pass over all files? In most cases, this would mean that it makes twice as many passes as needed. Isn't that a fairly large performance overhead?

@lucacasonato
Copy link
Member

deno fmt does not exhibit this behaviour anymore because we format until the result is stable. Upstream bug is tracked in dprint/dprint-plugin-typescript#670

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly deno fmt Related to the "deno fmt" subcommand or dprint
Projects
None yet
Development

No branches or pull requests

5 participants