Skip to content

refactor: update file_diff_contents to use git2 instead of CLI, make path helper more robust#283

Closed
Scott McMaster (scottmcmaster) wants to merge 4 commits into
scott-port-log-to-git2from
06-03-scott-port-file-diff-contents-to-git2
Closed

refactor: update file_diff_contents to use git2 instead of CLI, make path helper more robust#283
Scott McMaster (scottmcmaster) wants to merge 4 commits into
scott-port-log-to-git2from
06-03-scott-port-file-diff-contents-to-git2

Conversation

@scottmcmaster

@scottmcmaster Scott McMaster (scottmcmaster) commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

Port file_diff_contents over to git2 in the query submodule, also move associated tests. Create required new file operations in a new repo_files.rs file and add independent unit tests for them, Also make the path normalization helper more robust since the one that was there before looked like it might have a few robustness issues.

Test Plan

Added and updated unit tests, run evolve manually to trigger the diffing command.

  • No test plan needed

Docs

  • Docs updated (companion PR in darkmatter/nixmac-web: #___)
  • No docs update needed

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Scott McMaster (scottmcmaster) commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more


How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • automerge - adds this PR to the back of the merge queue
  • urgent - for urgent changes, fast-track this PR to the front of the merge queue

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

📋 PR Overview

Lines changed 338 (+282 / -56)
Files 1 added, 4 modified, 0 deleted
Draft / WIP no
Has Test Plan yes
No Test Plan Needed no
New UI components no
New Storybook stories no
New Rust modules yes (1)
New TS source files no
New tests no
package.json touched no
Cargo.toml touched no
Infra / CI touched no

🔬 Coverage

Report Lines Statements Functions Branches
apps/native/coverage/coverage-summary.json 26.3% 26.4% 24.2% 17.9%

Generated by 🚫 dangerJS against 452ba67

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors Git file-content diff retrieval to use git2 (instead of spawning the Git CLI), while hardening repository-relative path handling and centralizing repo file helpers in a dedicated module.

Changes:

  • Moved file_diff_contents into the git::query (git2-based) layer and updated the Tauri command to call it.
  • Added git::repo_files helpers for lexical path normalization plus reading HEAD/workdir contents, with new unit tests.
  • Removed the old CLI-based file_diff_contents implementation and its associated tests from git::exec.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
apps/native/src-tauri/src/git/repo_files.rs New helper module for safe-ish repo-relative path normalization and reading file contents from HEAD/workdir, plus unit tests.
apps/native/src-tauri/src/git/query.rs Adds git2-backed file_diff_contents and ports tests to the query layer.
apps/native/src-tauri/src/git/mod.rs Registers the new repo_files module.
apps/native/src-tauri/src/git/exec.rs Removes the old CLI-based file_diff_contents implementation and tests.
apps/native/src-tauri/src/commands/git.rs Updates the command to use git::query::file_diff_contents.

Comment thread apps/native/src-tauri/src/git/repo_files.rs Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!


/// Reads the contents of a file at the given path from the working directory of the repository.
/// Returns an empty string if the file does not exist on disk.
/// Symlinks are followed, but only if the resolved target remains inside the repository workdir.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, leaving this as a note-to-self, as this just got me thinking about symlinks that DO cross the repository boundary, since it is pretty common to symlink files to e.g. ~/.config/code/settings.json to sync vs code settings across machines.

@graphite-app graphite-app Bot changed the base branch from scott-port-log-to-git2 to graphite-base/283 June 5, 2026 00:19
@graphite-app

graphite-app Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jun 5, 12:29 AM UTC: czxtm added this pull request to the Graphite merge queue.
  • Jun 5, 12:30 AM UTC: CI is running for this pull request on a draft pull request (#318) due to your merge queue CI optimization settings.
  • Jun 5, 12:33 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #281.
  • Jun 5, 12:33 AM UTC: The Graphite merge queue removed this pull request due to downstack failures on PR #281.
  • Jun 5, 1:30 AM UTC: czxtm added this pull request to the Graphite merge queue.
  • Jun 5, 1:31 AM UTC: CI is running for this pull request on a draft pull request (#321) due to your merge queue CI optimization settings.
  • Jun 5, 1:37 AM UTC: Merged by the Graphite merge queue via draft PR: #321.

@scottmcmaster Scott McMaster (scottmcmaster) force-pushed the 06-03-scott-port-file-diff-contents-to-git2 branch from 452ba67 to 48f72d6 Compare June 5, 2026 00:46
@scottmcmaster Scott McMaster (scottmcmaster) changed the base branch from graphite-base/283 to scott-port-log-to-git2 June 5, 2026 00:46
@graphite-app graphite-app Bot closed this Jun 5, 2026
@graphite-app graphite-app Bot deleted the 06-03-scott-port-file-diff-contents-to-git2 branch June 5, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants