Skip to content
This repository has been archived by the owner on Aug 31, 2023. It is now read-only.

rome format . does not log to stderr #3168

Closed
1 task done
anonrig opened this issue Sep 6, 2022 · 9 comments
Closed
1 task done

rome format . does not log to stderr #3168

anonrig opened this issue Sep 6, 2022 · 9 comments
Labels
S-Wishlist Possible interesting features not on the current roadmap

Comments

@anonrig
Copy link
Contributor

anonrig commented Sep 6, 2022

Environment information

macos latest
rome 0.9.0
node 18.8.0

What happened?

rome format . does not print to stderr when ---write configuration is missing

➜  fast-querystring git:(feat/stringify) ✗ npm run format:ci

> fast-querystring@0.4.0 format:ci
> rome format .

./test/node.ts: help[Formatter]: Formatter would have printed the following content:
      | @@ -23,7 +23,7 @@
22 22 |       { frappucino: "muffin", "goat[]": "scone", pond: "moose" },
23 23 |     ],
24 24 |     ["trololol=yes&lololo=no", { trololol: "yes", lololo: "no" }],
25    | - ]
   25 | + ];
26 26 |   export const qsTestCases = [
27 27 |     [
28 28 |       "__proto__=1",

Compared 9 files in 1626µs

Expected result

It should log to stderr

Code of Conduct

  • I agree to follow Rome's Code of Conduct
@anonrig anonrig added the S-To triage Status: user report of a possible bug that needs to be triaged label Sep 6, 2022
@anonrig
Copy link
Contributor Author

anonrig commented Sep 6, 2022

Check linter step on the example failing CI: https://github.com/anonrig/fast-querystring/pull/10/checks

@ematipico
Copy link
Contributor

ematipico commented Sep 7, 2022

Hi @anonrig , how did you verify that diagnostics are not wrote on stderr? I can't reproduce it.

I just created a file with some errors, run rome ci teset.js &> errors.txt and I can correctly see that this file shows the diagnostics.

@leops
Copy link
Contributor

leops commented Sep 7, 2022

I think this is intentional, rome ci emits an error if the current content of the file differs from the output of the formatter and this correctly gets printed to stderr, but rome format only emits an informational message that displays the diff to the console so that gets printed to stdout since it's not an error (it also doesn't cause the CLI to exit with a non-zero result code)

@anonrig
Copy link
Contributor Author

anonrig commented Sep 7, 2022

rome format only emits an informational message that displays the diff to the console so that gets printed to stdout since it's not an error (it also doesn't cause the CLI to exit with a non-zero result code)

I expected --write to give stdout and stderr output without using it. Can you add a footnote to the rome format documentation regarding it?

rome ci forces the developer to use rome.json and I just want to run run format and throw an error if it fails.

@ematipico
Copy link
Contributor

rome ci forces the developer to use rome.json and I just want to run run format and throw an error if it fails.

That's a good use case. What do you think if provide arguments to disable specific tools? Something like:

  • rome ci --format-only
  • rome ci --lint-only

@anonrig
Copy link
Contributor Author

anonrig commented Sep 7, 2022

What about removing rome ci and just have a rome format --fail-on-error or --ci kind of flag?

@MichaReiser MichaReiser added S-Wishlist Possible interesting features not on the current roadmap and removed S-To triage Status: user report of a possible bug that needs to be triaged labels Sep 8, 2022
@github-actions
Copy link

This issue is stale because it has been open 14 days with no activity.

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Sep 23, 2022
@anonrig
Copy link
Contributor Author

anonrig commented Sep 23, 2022

image

@ematipico
Copy link
Contributor

image

Don't worry :) we are going to track this feature in a separate issue as we are about to plan our next release cycle.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Wishlist Possible interesting features not on the current roadmap
Projects
None yet
Development

No branches or pull requests

4 participants