deno icon indicating copy to clipboard operation
deno copied to clipboard

Confirm `deno fmt` in well know directories

Open irbull opened this issue 1 year ago • 2 comments

deno fmt is a pretty dangerous command as it will format all the files in a given directory (and sub directories). By default, it will update all files in place. Other tools, such as cargo fmt check for the existence of configuration files (such as a Cargo.toml), but Deno doesn't require any such files.

If deno fmt is run from the root directory, it could cause significant damage to a users file system, and it appears there was one such instance of this that was reported on the Discord channel.

The deno fmt command should ask for confirmation if run in well known (Suspicious) directories that are likely not Deno directories. In particular, if running in a tty environment, deno should ask the user to confirm in the following directories:

  • ~/.
  • ~/Downloads
  • ~/Documents
  • ~/Desktop
  • /
  • ~/git

Thoughts?

irbull avatar Nov 04 '24 04:11 irbull

I think instead of seeing certain directories as special, it should instead look for the existence of a deno.json, package.json or jsr.json. If none of these exist then it should prompt for confirmation. A flag such as --skip-confirmation could be passed explicitly for skipping the double confirmation in directories without these files if the user so desired.

Even though deno is designed to work without a config file, it's rare that it would actually be used without one and a double confirmation for those situations and a flag if they're setting up say a Makefile would be beneficial overall.

BlackAsLight avatar Nov 04 '24 07:11 BlackAsLight

@BlackAsLight I really like that suggestion. I was a bit worried that it might break existing workflows, especially the use of deno fmt from VSCode, but the LSP could be adjusted to always use the --skip-confirmation flag.

I've noticed that Deno often takes inspiration from Rust / Cargo, and if we ask ourselves What would Rust do? then we arrive at the suggestion @BlackAsLight made. If there is no configuration file in the current directory (or in a parent directory all the way to the root) then the command fails (with status code '1') and the following error is printed:

`cargo metadata` exited with an error: error: could not find `Cargo.toml` in `/Users/irbull/git/deno/mono` or any parent directory

This utility formats all bin and lib files of the current crate using rustfmt.

Usage: cargo fmt [OPTIONS] [-- <rustfmt_options>...]
...

This suggestion shouldn't break any CI jobs, as you can't really format code during a CI/CD pipeline (you can check formatting, but that won't change).

So what do we think about this:

  • Adjust the CLI to only automatically format code if a package.json, deno.json OR deno.jsonc file is present in the current directory (or parent directory)
  • If skip-confirmation is passed, always format without confirmation
  • Update the LSP to use skip-confirmation by default, so auto format works in VSCode

David probably has the most experience here as he authored DPrint. Maybe he has some thoughts.

irbull avatar Nov 04 '24 16:11 irbull

See https://github.com/denoland/deno/pull/30623 for a possible solution

dsherret avatar Sep 05 '25 16:09 dsherret