format: replace style.fish by xtask

Replace the `build_tools/style.fish` script by an xtask. This eliminates
the need for a fish binary for performing the formatting/checking. The
`fish_indent` binary is still needed. Eventually, this should be made
available as a library function, so the xtask can use that instead of
requiring a `fish_indent` binary in the `$PATH`.

The new xtask is called `format` rather than `style`, because that's a
more fitting description of what it does (and what the script it
replaces did).

The old script's behavior is not replicated exactly:
- Specifying `--all` and explicit paths is supported within a single
  invocation.
- Explicit arguments no longer have to be files. If a directory is
  specified, all files within it will be considered.
- The git check for un-staged changes is no longer filtered by file
  names, mainly to simplify the implementation.
- A warning is now printed if neither the `--all` flag nor a path are
  provided as arguments. The reason for this is that one might assume
  that omitting these arguments would default to formatting everything
  in the current directory, but instead no formatting will happen in
  this case.
- The wording of some messages is different.

The design of the new code tries to make it easy to add formatters for
additional languages, or change the ones we already have. This is
achieved by separating the code into one function per language, which
can be modified without touching the code for the other languages.
Adding support for a new formatter/language only requires adding a
function which builds the formatter command line based on the arguments
to the xtask, and calling that function from the main `format` function.

Closes #12467

.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
 

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
 -----------------------

Cargo.lock

@@ -1148,9 +1148,11 @@ checksum = "f17a85883d4e6d00e8a97c586de764dabcc06133f7f1d55dce5cdc070ad7fe59"
 name = "xtask"
 version = "0.0.0"
 dependencies = [
+ "anstyle",
  "clap",
  "fish-build-helper",
  "fish-tempfile",
+ "walkdir",
 ]
 
 [[package]]

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]

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

build_tools/style.fish

@@ -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 <Cargo.toml)" = "$($edition_spec <.rustfmt.toml)"
-    or die "Cargo.toml and .rustfmt.toml use different editions"
-
-    echo === Running "$green"rustfmt"$normal"
-    if set -l -q _flag_check
-        if test $all = yes
-            cargo fmt --all --check
-        else
-            rustfmt --check --files-with-diff $rust_files
-        end
-        or die "Rust files are not formatted correctly."
-    else
-        if test $all = yes
-            cargo fmt --all
-        else
-            rustfmt $rust_files
-        end
-    end
-end

crates/xtask/Cargo.toml

@@ -6,6 +6,8 @@ edition.workspace = true
 repository.workspace = true
 
 [dependencies]
+anstyle.workspace = true
 clap.workspace = true
 fish-build-helper.workspace = true
 fish-tempfile.workspace = true
+walkdir.workspace = true

crates/xtask/src/format.rs

@@ -0,0 +1,195 @@
+use anstyle::{AnsiColor, Style};
+use clap::Args;
+use std::{
+    io::{ErrorKind, Write},
+    path::{Path, PathBuf},
+    process::{Command, Stdio},
+};
+use walkdir::WalkDir;
+
+const GREEN: Style = AnsiColor::Green.on_default();
+const YELLOW: Style = AnsiColor::Yellow.on_default();
+
+#[derive(Args)]
+pub struct FormatArgs {
+    /// Consider all eligible files.
+    #[arg(long)]
+    all: bool,
+    /// Report files which are not formatted as expected, without modifying any files.
+    #[arg(long)]
+    check: bool,
+    /// Format files even if uncommitted changes are detected.
+    #[arg(long)]
+    force: bool,
+    paths: Vec<PathBuf>,
+}
+
+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<P: AsRef<Path>, I: IntoIterator<Item = P>, M: Fn(&Path) -> bool>(
+    all_paths: I,
+    matcher: M,
+) -> Vec<PathBuf> {
+    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<P: AsRef<Path>, I: IntoIterator<Item = P>>(
+    all_paths: I,
+    extension: &str,
+) -> Vec<PathBuf> {
+    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");
+    }
+}

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);
 }

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"]),
     }