diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 385642249..83a64da0e 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -16,9 +16,9 @@ jobs: - name: install dependencies run: pip install ruff - name: build fish - run: cargo build + run: cargo build --bin fish_indent - name: check format - run: PATH="target/debug:$PATH" build_tools/style.fish --all --check + run: PATH="target/debug:$PATH" cargo xtask format --all --check - name: check rustfmt run: find build.rs crates src -type f -name '*.rs' | xargs rustfmt --check diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 920fd2843..d79c41948 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -133,12 +133,12 @@ For formatting, we use: - ``fish_indent`` (shipped with fish) for fish script - ``ruff format`` for Python -To reformat files, there is a script +To reformat files, there is an xtask :: - build_tools/style.fish --all - build_tools/style.fish somefile.rs some.fish + cargo xtask format --all + cargo xtask format somefile.rs some.fish Fish Script Style Guide ----------------------- diff --git a/Cargo.lock b/Cargo.lock index b36c8215c..b371b3b5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1148,9 +1148,11 @@ checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59" name = "xtask" version = "0.0.0" dependencies = [ + "anstyle", "clap", "fish-build-helper", "fish-tempfile", + "walkdir", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index d89f83d02..14cc50478 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ repository = "https://github.com/fish-shell/fish-shell" license = "GPL-2.0-only AND LGPL-2.0-or-later AND MIT AND PSF-2.0" [workspace.dependencies] +anstyle = "1.0.13" assert_matches = "1.5.0" bitflags = "2.5.0" cc = "1.0.94" @@ -76,6 +77,7 @@ widestring = "1.2.0" unicode-segmentation = "1.12.0" unicode-width = "0.2.0" unix_path = "1.0.1" +walkdir = "2.5.0" xterm-color = "1.0.1" [profile.release] diff --git a/build_tools/check.sh b/build_tools/check.sh index 4f70e035e..c3fbf577e 100755 --- a/build_tools/check.sh +++ b/build_tools/check.sh @@ -78,7 +78,7 @@ if $lint; then if command -v cargo-deny >/dev/null; then cargo deny --all-features --locked --exclude-dev check licenses fi - PATH="$build_dir:$PATH" "$workspace_root/build_tools/style.fish" --all --check + PATH="$build_dir:$PATH" cargo xtask format --all --check for features in "" --no-default-features; do cargo clippy --workspace --all-targets $features done diff --git a/build_tools/style.fish b/build_tools/style.fish deleted file mode 100755 index b8fd1f8a0..000000000 --- a/build_tools/style.fish +++ /dev/null @@ -1,123 +0,0 @@ -#!/usr/bin/env fish -# -# This runs Python files, fish scripts (*.fish), and Rust files -# through their respective code formatting programs. -# -# `--all`: Format all eligible files instead of the ones specified as arguments. -# `--check`: Instead of reformatting, fail if a file is not formatted correctly. -# `--force`: Proceed without asking if uncommitted changes are detected. -# Only relevant if `--all` is specified but `--check` is not specified. - -set -l fish_files -set -l python_files -set -l rust_files -set -l all no - -argparse all check force -- $argv -or exit $status - -if set -l -q _flag_all - set all yes - if set -q argv[1] - echo "Unexpected arguments: '$argv'" - exit 1 - end -end - -set -l workspace_root (realpath (status dirname)/..) - -if test $all = yes - if not set -l -q _flag_force; and not set -l -q _flag_check - # Potential for false positives: Not all fish files are formatted, see the `fish_files` - # definition below. - set -l relevant_uncommitted_changes (git status --porcelain --short --untracked-files=all | sed -e 's/^ *[^ ]* *//' | grep -E '.*\.(fish|py|rs)$') - if set -q relevant_uncommitted_changes[1] - for changed_file in $relevant_uncommitted_changes - echo $changed_file - end - echo - echo 'You have uncommitted changes (listed above). Are you sure you want to restyle?' - read -P 'y/N? ' -n1 -l ans - if not string match -qi y -- $ans - exit 1 - end - end - end - set fish_files $workspace_root/{benchmarks,build_tools,etc,share}/**.fish - set python_files $workspace_root -else - # Format the files specified as arguments. - set -l files $argv - set fish_files (string match -r '^.*\.fish$' -- $files) - set python_files (string match -r '^.*\.py$' -- $files) - set rust_files (string match -r '^.*\.rs$' -- $files) -end - -set -l red (set_color red) -set -l green (set_color green) -set -l yellow (set_color yellow) -set -l normal (set_color normal) - -function die -V red -V normal - echo $red$argv[1]$normal - exit 1 -end - -if set -q fish_files[1] - if not type -q fish_indent - echo - echo $yellow'Could not find `fish_indent` in `$PATH`.'$normal - exit 127 - end - echo === Running "$green"fish_indent"$normal" - if set -l -q _flag_check - fish_indent --check -- $fish_files - or die "Fish files are not formatted correctly." - else - fish_indent -w -- $fish_files - end -end - -if set -q python_files[1] - if not type -q ruff - echo - echo $yellow'Please install `ruff` to style python'$normal - exit 127 - end - echo === Running "$green"ruff format"$normal" - if set -l -q _flag_check - ruff format --check $python_files - or die "Python files are not formatted correctly." - else - ruff format $python_files - end -end - -if test $all = yes; or set -q rust_files[1] - if not cargo fmt --version >/dev/null - echo - echo $yellow'Please install "rustfmt" to style Rust, e.g. via:' - echo "rustup component add rustfmt"$normal - exit 127 - end - - set -l edition_spec string match -r '^edition\s*=.*' - test "$($edition_spec , +} + +pub fn format(args: FormatArgs) { + if !args.all && args.paths.is_empty() { + println!( + "{YELLOW}warning: No paths specified. Nothing to do. Use the \"--all\" flag to consider all eligible files.{YELLOW:#}" + ); + return; + } + if !args.force && !args.check { + match Command::new("git") + .args(["status", "--porcelain", "--short", "--untracked-files=all"]) + .output() + { + Ok(output) => { + if !output.stdout.is_empty() { + std::io::stdout().write_all(&output.stdout).unwrap(); + print!( + "You have uncommitted changes (listed above). Are you sure you want to format? (y/N): " + ); + std::io::stdout().flush().unwrap(); + let mut response = String::new(); + std::io::stdin().read_line(&mut response).unwrap(); + if response.trim_end() != "y" { + println!("Exiting without formatting."); + return; + } + } + } + Err(e) => { + if e.kind() == ErrorKind::NotFound { + println!( + "{YELLOW}warning: Did not find git, will proceed without checking for unstaged changes.{YELLOW:#}" + ) + } else { + panic!("Failed to run git status:\n{e}") + } + } + } + } + format_fish(&args); + format_python(&args); + format_rust(&args); +} + +fn get_matching_files, I: IntoIterator, M: Fn(&Path) -> bool>( + all_paths: I, + matcher: M, +) -> Vec { + all_paths + .into_iter() + .flat_map(WalkDir::new) + .filter_map(|res| { + let entry = res.unwrap(); + let path = entry.path(); + if entry.file_type().is_file() && matcher(path) { + Some(path.to_owned()) + } else { + None + } + }) + .collect() +} +fn files_with_extension, I: IntoIterator>( + all_paths: I, + extension: &str, +) -> Vec { + let matcher = |p: &Path| p.extension().is_some_and(|e| e == extension); + get_matching_files(all_paths, matcher) +} + +fn run_formatter(formatter: &mut Command, name: &str) { + println!("=== Running {GREEN}{name}{GREEN:#}"); + match formatter.status() { + Ok(exit_status) => { + if !exit_status.success() { + panic!("{name:?}: Files are not formatted correctly."); + } + } + Err(e) => { + if e.kind() == std::io::ErrorKind::NotFound { + eprintln!( + "{YELLOW}Formatter not found: {name:?}. Skipping associated files.{YELLOW:#}" + ); + } else { + panic!("Error occurred while running {name:?}:\n{e}") + } + } + } +} + +fn format_fish(args: &FormatArgs) { + let mut fish_paths = files_with_extension(&args.paths, "fish"); + if args.all { + let workspace_root = fish_build_helper::workspace_root(); + let fish_formatting_dirs = ["benchmarks", "build_tools", "etc", "share"]; + fish_paths.extend(files_with_extension( + fish_formatting_dirs + .iter() + .map(|dir_name| workspace_root.join(dir_name)), + "fish", + )); + }; + if fish_paths.is_empty() { + return; + } + // TODO: make `fish_indent` available as a Rust library function, to avoid needing a + // `fish_indent` binary in `$PATH`. + let mut formatter = Command::new("fish_indent"); + if args.check { + formatter.arg("--check"); + } else { + formatter.arg("-w"); + } + formatter.arg("--"); + formatter.args(fish_paths); + run_formatter(&mut formatter, "fish_indent"); +} + +fn format_python(args: &FormatArgs) { + let mut formatter = Command::new("ruff"); + formatter.arg("format"); + if args.check { + formatter.arg("--check"); + } + let mut python_files = files_with_extension(&args.paths, "py"); + + if args.all { + python_files.push(fish_build_helper::workspace_root().to_owned()); + }; + if python_files.is_empty() { + return; + } + formatter.args(python_files); + run_formatter(&mut formatter, "ruff format"); +} + +fn format_rust(args: &FormatArgs) { + let rustfmt_status = Command::new("cargo") + .arg("fmt") + .arg("--version") + .stdout(Stdio::null()) + .status() + .unwrap(); + if !rustfmt_status.success() { + eprintln!( + "{YELLOW}Please install \"rustfmt\" to format Rust, e.g. via:\n\ + rustup component add rustfmt{YELLOW:#}" + ); + return; + } + if args.all { + let mut formatter = Command::new("cargo"); + formatter.arg("fmt"); + formatter.arg("--all"); + if args.check { + formatter.arg("--check"); + } + run_formatter(&mut formatter, "cargo fmt"); + } + let rust_files = files_with_extension(&args.paths, "rs"); + if !rust_files.is_empty() { + let mut formatter = Command::new("rustfmt"); + if args.check { + formatter.arg("--check"); + formatter.arg("--files-with-diff"); + } + formatter.args(rust_files); + run_formatter(&mut formatter, "rustfmt"); + } +} diff --git a/crates/xtask/src/lib.rs b/crates/xtask/src/lib.rs index f6c0d3f22..e2f23eb8a 100644 --- a/crates/xtask/src/lib.rs +++ b/crates/xtask/src/lib.rs @@ -1,5 +1,7 @@ use std::{ffi::OsStr, process::Command}; +pub mod format; + pub trait CommandExt { fn run_or_panic(&mut self); } diff --git a/crates/xtask/src/main.rs b/crates/xtask/src/main.rs index ee7b1c4e0..af405cd69 100644 --- a/crates/xtask/src/main.rs +++ b/crates/xtask/src/main.rs @@ -1,7 +1,7 @@ use clap::{Parser, Subcommand}; use fish_build_helper::as_os_strs; use std::{path::PathBuf, process::Command}; -use xtask::{CommandExt as _, cargo}; +use xtask::{CommandExt as _, cargo, format::FormatArgs}; #[derive(Parser)] #[command( @@ -18,6 +18,8 @@ struct Cli { enum Task { /// Run various checks on the repo. Check, + /// Format files or check if they are correctly formatted. + Format(FormatArgs), /// Build HTML docs HtmlDocs { /// Path to a fish_indent executable. If none is specified, fish_indent will be built. @@ -32,6 +34,7 @@ fn main() { let cli = Cli::parse(); match cli.task { Task::Check => run_checks(), + Task::Format(format_args) => xtask::format::format(format_args), Task::HtmlDocs { fish_indent } => build_html_docs(fish_indent), Task::ManPages => cargo(["build", "--package", "fish-build-man-pages"]), }