From 27795be5a7a559731ad08a16c9e18c5c03412a4a Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:49:20 +0800 Subject: [PATCH] Remove get prefixes from Git helpers --- crates/prek/src/cli/hook_impl.rs | 6 +- crates/prek/src/cli/install.rs | 8 +-- crates/prek/src/cli/run/filter.rs | 6 +- crates/prek/src/cli/try_repo.rs | 2 +- crates/prek/src/git.rs | 72 ++++++++++--------- .../check_added_large_files.rs | 6 +- .../pre_commit_hooks/check_case_conflict.rs | 2 +- .../pre_commit_hooks/check_merge_conflict.rs | 4 +- crates/prek/tests/builtin_hooks.rs | 6 +- 9 files changed, 59 insertions(+), 53 deletions(-) diff --git a/crates/prek/src/cli/hook_impl.rs b/crates/prek/src/cli/hook_impl.rs index a1cf5f139..fbf12c9a0 100644 --- a/crates/prek/src/cli/hook_impl.rs +++ b/crates/prek/src/cli/hook_impl.rs @@ -344,7 +344,7 @@ async fn parse_pre_push_info(remote_name: &str, stdin: &[u8]) -> Result Result Result = if all { diff --git a/crates/prek/src/cli/run/filter.rs b/crates/prek/src/cli/run/filter.rs index f00b393eb..4d5c2e15f 100644 --- a/crates/prek/src/cli/run/filter.rs +++ b/crates/prek/src/cli/run/filter.rs @@ -667,7 +667,7 @@ async fn collect_files_for_selection( ) -> Result> { match selection { FileSelection::Diff { from_ref, to_ref } => { - let files = git::get_changed_files(&from_ref, &to_ref, workspace_root).await?; + let files = git::changed_files(&from_ref, &to_ref, workspace_root).await?; debug!( "Files changed between {} and {}: {}", from_ref, @@ -688,12 +688,12 @@ async fn collect_files_for_selection( } FileSelection::Default => { if git::is_in_merge_conflict().await? { - let files = git::get_conflicted_files(workspace_root).await?; + let files = git::conflicted_files(workspace_root).await?; debug!("Conflicted files: {}", files.len()); return Ok(files); } - let files = git::get_staged_files(workspace_root).await?; + let files = git::staged_files(workspace_root).await?; debug!("Staged files: {}", files.len()); Ok(files) } diff --git a/crates/prek/src/cli/try_repo.rs b/crates/prek/src/cli/try_repo.rs index 3b43d1840..2b71c4d64 100644 --- a/crates/prek/src/cli/try_repo.rs +++ b/crates/prek/src/cli/try_repo.rs @@ -51,7 +51,7 @@ async fn clone_and_commit(repo_path: &Path, head_rev: &str, tmp_dir: &Path) -> R let index_path = shadow.join(".git/index"); let objects_path = shadow.join(".git/objects"); - let staged_files = git::get_staged_files(repo_path).await?; + let staged_files = git::staged_files(repo_path).await?; if !staged_files.is_empty() { git::git_cmd()? .arg("add") diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index 9be568f14..87a17d8ac 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -62,7 +62,7 @@ fn git_work_tree() -> Option<&'static Path> { } pub(crate) static GIT_ROOT: LazyLock> = LazyLock::new(|| { - get_root() + root() .map(|root| dunce::canonicalize(&root).unwrap_or(root)) .inspect(|root| { debug!("Git root: {}", root.display()); @@ -172,7 +172,7 @@ pub(crate) async fn intent_to_add_files(root: &Path) -> Result, Err Ok(zsplit(&output.stdout)?) } -pub(crate) async fn get_added_files(root: &Path) -> Result, Error> { +pub(crate) async fn staged_added_files(root: &Path) -> Result, Error> { let output = git_cmd()? .current_dir(root) .arg("diff") @@ -190,7 +190,7 @@ pub(crate) async fn get_added_files(root: &Path) -> Result, Error> Ok(zsplit(&output.stdout)?) } -pub(crate) async fn get_changed_files( +pub(crate) async fn changed_files( old: &str, new: &str, root: &Path, @@ -248,7 +248,7 @@ where Ok(zsplit(&output.stdout)?) } -pub(crate) async fn get_git_dir() -> Result { +pub(crate) async fn git_dir() -> Result { let output = git_cmd()? .arg("rev-parse") .arg("--git-dir") @@ -260,7 +260,7 @@ pub(crate) async fn get_git_dir() -> Result { )) } -pub(crate) async fn get_git_common_dir() -> Result { +pub(crate) async fn common_dir() -> Result { let output = git_cmd()? .arg("rev-parse") .arg("--git-common-dir") @@ -268,7 +268,7 @@ pub(crate) async fn get_git_common_dir() -> Result { .output() .await?; if output.stdout.trim_ascii().is_empty() { - Ok(get_git_dir().await?) + Ok(git_dir().await?) } else { Ok(PathBuf::from( String::from_utf8_lossy(&output.stdout).trim_ascii(), @@ -276,7 +276,7 @@ pub(crate) async fn get_git_common_dir() -> Result { } } -pub(crate) async fn get_git_hooks_dir() -> Result { +pub(crate) async fn hooks_dir() -> Result { // Ask Git for the effective hooks directory instead of reconstructing it // ourselves. That lets Git apply the full precedence chain for // `core.hooksPath`, including local/worktree config, linked worktrees, bare @@ -290,7 +290,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result { .output() .await?; let hooks_dir = if output.stdout.trim_ascii().is_empty() { - get_git_common_dir().await?.join("hooks") + common_dir().await?.join("hooks") } else { PathBuf::from(String::from_utf8_lossy(&output.stdout).trim_ascii()) }; @@ -308,7 +308,7 @@ pub(crate) async fn get_git_hooks_dir() -> Result { } } -pub(crate) async fn get_staged_files(root: &Path) -> Result, Error> { +pub(crate) async fn staged_files(root: &Path) -> Result, Error> { let output = git_cmd()? .current_dir(root) .arg("diff") @@ -365,11 +365,11 @@ pub(crate) async fn has_diff(rev: &str, path: &Path) -> Result { } pub(crate) async fn is_in_merge_conflict() -> Result { - let git_dir = get_git_dir().await?; + let git_dir = git_dir().await?; Ok(git_dir.join("MERGE_HEAD").try_exists()? && git_dir.join("MERGE_MSG").try_exists()?) } -pub(crate) async fn get_conflicted_files(root: &Path) -> Result, Error> { +pub(crate) async fn conflicted_files(root: &Path) -> Result, Error> { let tree = git_cmd()?.arg("write-tree").check(true).output().await?; let output = git_cmd()? @@ -396,7 +396,7 @@ pub(crate) async fn get_conflicted_files(root: &Path) -> Result, Er } async fn parse_merge_msg_for_conflicts() -> Result, Error> { - let git_dir = get_git_dir().await?; + let git_dir = git_dir().await?; let merge_msg = git_dir.join("MERGE_MSG"); let content = fs_err::tokio::read_to_string(&merge_msg).await?; let conflicts = content @@ -471,9 +471,9 @@ pub(crate) async fn write_tree() -> Result { .to_string()) } -/// Get the path of the top-level directory of the working tree. +/// Return the path of the top-level directory of the working tree. #[instrument(level = "trace")] -pub(crate) fn get_root() -> Result { +pub(crate) fn root() -> Result { let git = GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?; let mut cmd = Command::new(git); let output = apply_git_work_tree(&mut cmd) @@ -709,7 +709,7 @@ pub(crate) async fn clone_repo( clone_repo_attempt(rev, path, terminal_prompt).await } -async fn get_config_value(scope: Option<&str>, key: &str) -> Result>, Error> { +async fn config_value(scope: Option<&str>, key: &str) -> Result>, Error> { let mut cmd = git_cmd()?; cmd.arg("config").arg("--includes"); if let Some(scope) = scope { @@ -729,11 +729,11 @@ async fn has_config_value(scope: Option<&str>, key: &str) -> Result // An empty config value still counts as configured and can affect Git's // path resolution, e.g. `core.hooksPath=` makes `--git-path hooks` // resolve to the current directory. - Ok(get_config_value(scope, key).await?.is_some()) + Ok(config_value(scope, key).await?.is_some()) } async fn config_value_is_empty(scope: Option<&str>, key: &str) -> Result { - Ok(get_config_value(scope, key) + Ok(config_value(scope, key) .await? .as_deref() .is_some_and(|value| value.strip_suffix(b"\0").unwrap_or(value).is_empty())) @@ -752,7 +752,7 @@ pub(crate) async fn has_repo_hooks_path_set() -> Result { /// /// This mirrors the relevant parts of Git's `git_config_perm` in `setup.c` /// and `calc_shared_perm` in `path.c`. -fn shared_repository_file_mode(value: &str, mode: u32) -> Option { +fn apply_shared_repository_file_mode(value: &str, mode: u32) -> Option { const PERM_GROUP: u32 = 0o660; const PERM_EVERYBODY: u32 = 0o664; @@ -797,7 +797,7 @@ fn shared_repository_file_mode(value: &str, mode: u32) -> Option { } /// Resolve the file mode implied by `core.sharedRepository` for a newly created file. -pub(crate) async fn get_shared_repository_file_mode(mode: u32) -> Result { +pub(crate) async fn shared_repository_file_mode(mode: u32) -> Result { let output = git_cmd()? .arg("config") .arg("--get") @@ -807,13 +807,13 @@ pub(crate) async fn get_shared_repository_file_mode(mode: u32) -> Result { .await?; if output.status.success() { let value = str::from_utf8(&output.stdout)?; - Ok(shared_repository_file_mode(value, mode).unwrap_or(mode)) + Ok(apply_shared_repository_file_mode(value, mode).unwrap_or(mode)) } else { Ok(mode) } } -pub(crate) async fn get_lfs_files( +pub(crate) async fn lfs_files( current_dir: &Path, paths: &[&Path], ) -> Result, Error> { @@ -911,8 +911,8 @@ pub(crate) async fn is_ancestor(ancestor: &str, commit: &str) -> Result Result, Error> { @@ -933,8 +933,8 @@ pub(crate) async fn get_ancestors_not_in_remote( .collect()) } -/// Get root commits (commits with no parents) for the given commit -pub(crate) async fn get_root_commits(local_sha: &str) -> Result, Error> { +/// Return root commits (commits with no parents) for the given commit. +pub(crate) async fn root_commits(local_sha: &str) -> Result, Error> { let output = git_cmd()? .arg("rev-list") .arg("--max-parents=0") @@ -949,8 +949,8 @@ pub(crate) async fn get_root_commits(local_sha: &str) -> Result Result, Error> { +/// Return the parent commit of the given commit. +pub(crate) async fn parent_commit(commit: &str) -> Result, Error> { let output = git_cmd()? .arg("rev-parse") .arg(format!("{commit}^")) @@ -997,7 +997,7 @@ mod tests { #[cfg(unix)] use super::zsplit; use super::{ - Error, GIT, TerminalPrompt, full_clone, init_repo, shared_repository_file_mode, + Error, GIT, TerminalPrompt, apply_shared_repository_file_mode, full_clone, init_repo, should_update_submodules, update_submodules, }; use assert_cmd::assert::OutputAssertExt; @@ -1107,27 +1107,33 @@ mod tests { #[test] fn shared_repository_group_mode_matches_git_behavior() { for value in ["group", "true", "yes", "on", "1"] { - assert_eq!(shared_repository_file_mode(value, 0o755), Some(0o775)); + assert_eq!(apply_shared_repository_file_mode(value, 0o755), Some(0o775)); } } #[test] fn shared_repository_everybody_mode_matches_git_behavior() { for value in ["all", "world", "everybody", "2"] { - assert_eq!(shared_repository_file_mode(value, 0o755), Some(0o775)); + assert_eq!(apply_shared_repository_file_mode(value, 0o755), Some(0o775)); } } #[test] fn shared_repository_octal_mode_matches_git_behavior() { - assert_eq!(shared_repository_file_mode("0640", 0o644), Some(0o640)); - assert_eq!(shared_repository_file_mode("0640", 0o755), Some(0o750)); + assert_eq!( + apply_shared_repository_file_mode("0640", 0o644), + Some(0o640) + ); + assert_eq!( + apply_shared_repository_file_mode("0640", 0o755), + Some(0o750) + ); } #[test] fn shared_repository_umask_or_invalid_values_do_not_override_mode() { for value in ["", "umask", "false", "no", "off", "0", "invalid", "0400"] { - assert_eq!(shared_repository_file_mode(value, 0o755), None); + assert_eq!(apply_shared_repository_file_mode(value, 0o755), None); } } } diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs b/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs index 90961fc08..364978a5f 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_added_large_files.rs @@ -3,7 +3,7 @@ use std::path::{Path, PathBuf}; use clap::Parser; use rustc_hash::FxHashSet; -use crate::git::{get_added_files, get_lfs_files}; +use crate::git::{lfs_files, staged_added_files}; use crate::hook::Hook; use crate::hooks::HookOutput; use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; @@ -35,7 +35,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> anyhow::Result>(); @@ -53,7 +53,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> anyhow::Result Result } // Get relevant files (filenames + added files) and include their parent directories. - let added = git::get_added_files(work_dir).await?; + let added = git::staged_added_files(work_dir).await?; let mut relevant_files_with_dirs: FxHashSet<&Path> = FxHashSet::default(); for filename in &filenames { insert_path_and_parents(&mut relevant_files_with_dirs, filename); diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs b/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs index 728178d0e..fcbd3e037 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_merge_conflict.rs @@ -5,7 +5,7 @@ use anyhow::Result; use clap::Parser; use tokio::io::AsyncBufReadExt; -use crate::git::get_git_dir; +use crate::git::git_dir; use crate::hook::Hook; use crate::hooks::HookOutput; use crate::hooks::pre_commit_hooks::{hook_filenames, parse_hook_args}; @@ -48,7 +48,7 @@ pub(crate) async fn run(hook: &Hook, filenames: &[&Path]) -> Result async fn is_in_merge() -> Result { // Change directory temporarily or ensure we're in the right directory - let git_dir = get_git_dir().await?; + let git_dir = git_dir().await?; // Check if MERGE_MSG exists let merge_msg_exists = git_dir.join("MERGE_MSG").exists(); diff --git a/crates/prek/tests/builtin_hooks.rs b/crates/prek/tests/builtin_hooks.rs index d6c51a42d..60534fcaa 100644 --- a/crates/prek/tests/builtin_hooks.rs +++ b/crates/prek/tests/builtin_hooks.rs @@ -3367,9 +3367,9 @@ fn check_case_conflict_workspace_mode_includes_added_files() -> Result<()> { app.child("FOO.txt").write_str("conflicting case")?; context.git_add("app/FOO.txt"); - // Regression: in workspace mode, `get_added_files()` must return paths relative to the - // nested project root so added files still participate in conflict detection even when - // `--files` only names some other file in that project. + // Regression: in workspace mode, staged additions must be reported relative to the nested + // project root so they still participate in conflict detection even when `--files` only + // names some other file in that project. cmd_snapshot!( context.filters(), context