diff options
| author | Craig Jennings <c@cjennings.net> | 2026-05-03 20:11:49 -0500 |
|---|---|---|
| committer | Craig Jennings <c@cjennings.net> | 2026-05-03 20:11:49 -0500 |
| commit | 570b91b74ac981d392f1724d6011d0558b7150c2 (patch) | |
| tree | cc0f1a97309e5a4ce798f2fee974dcf749640900 /modules/coverage-core.el | |
| parent | f3d50d1b2aea4f1eda436c54a15cc18596f2df0b (diff) | |
| download | dotemacs-570b91b74ac981d392f1724d6011d0558b7150c2.tar.gz dotemacs-570b91b74ac981d392f1724d6011d0558b7150c2.zip | |
refactor: invoke git via argv in coverage diff helpers
`coverage-core.el` was running git through `shell-command-to-string`, which has two practical problems for central tooling: shell parsing surfaces (especially the `$(git merge-base ...)` substitution), and silent failure modes when git exits non-zero (the bad output just becomes empty parse results).
I extracted three small helpers. `cj/--coverage-git-string` runs git via `process-file` against a temp buffer and signals `user-error` on non-zero exit, with the argv, status, and trimmed output included. `cj/--coverage-git-merge-base` does its own `git merge-base HEAD <base>` invocation. `cj/--coverage-git-diff` is the diff wrapper that always appends `--unified=0`.
`cj/--coverage-changed-lines` now uses `pcase` over the scope symbol and composes the helpers. Branch-vs-main and branch-vs-parent compute the merge-base in a separate call before running `git diff <merge-base>..HEAD`, with no shell substitution involved.
One behavior change is worth flagging. A git failure used to disappear into an empty hash table. It now signals a `user-error` with the failing command, exit status, and git's stderr output.
Tests: I added two argv-boundary cases (working-tree and branch-vs-parent both assert the exact argv list seen) plus a non-zero-exit case that asserts the user-error path. The existing `test-coverage-core--command.el` smoke test gets its `shell-command-to-string` stub upgraded to a `process-file` stub.
Diffstat (limited to 'modules/coverage-core.el')
| -rw-r--r-- | modules/coverage-core.el | 58 |
1 files changed, 44 insertions, 14 deletions
diff --git a/modules/coverage-core.el b/modules/coverage-core.el index b6723eca..93979530 100644 --- a/modules/coverage-core.el +++ b/modules/coverage-core.el @@ -4,7 +4,7 @@ ;;; Commentary: ;; Language-agnostic core for diff-aware coverage reporting. ;; -;; Reads an LCOV file, shells to git diff at a selectable scope, +;; Reads an LCOV file, invokes git diff at a selectable scope, ;; intersects the results, and displays a report buffer. Languages ;; plug in via the backend registry (see `cj/coverage-backends'). ;; @@ -13,6 +13,7 @@ ;;; Code: (require 'seq) +(require 'subr-x) (defvar cj/coverage-backends nil "Registry of coverage backends in priority order. @@ -196,25 +197,54 @@ empty hash table. Malformed hunk headers are skipped silently." (forward-line 1))) result)) +(defun cj/--coverage-git-string (&rest args) + "Run git with ARGS and return its stdout as a string. +Signals `user-error' when git exits non-zero." + (with-temp-buffer + (let ((status (apply #'process-file "git" nil (current-buffer) nil args)) + (output (buffer-string))) + (unless (zerop status) + (user-error "git %s failed with status %s: %s" + (string-join args " ") + status + (string-trim output))) + output))) + +(defun cj/--coverage-git-merge-base (base) + "Return the merge-base between HEAD and BASE." + (let ((merge-base (string-trim + (cj/--coverage-git-string "merge-base" "HEAD" base)))) + (unless (not (string-empty-p merge-base)) + (user-error "git merge-base HEAD %s returned no commit" base)) + merge-base)) + +(defun cj/--coverage-git-diff (&rest args) + "Return git diff output for ARGS plus --unified=0." + (apply #'cj/--coverage-git-string + (append (list "diff") args (list "--unified=0")))) + (defun cj/--coverage-changed-lines (scope &optional base) "Return a hash table of files to changed line numbers for SCOPE. SCOPE is one of the symbols `working-tree', `staged', `branch-vs-main', or `branch-vs-parent'. For `branch-vs-parent', BASE is the ref to compare against; if nil, falls back to the tracked upstream @{upstream}. Signals `user-error' for any other SCOPE." - (let ((cmd (cond - ((eq scope 'working-tree) - "git diff HEAD --unified=0") - ((eq scope 'staged) - "git diff --cached --unified=0") - ((eq scope 'branch-vs-main) - "git diff $(git merge-base HEAD main)..HEAD --unified=0") - ((eq scope 'branch-vs-parent) - (format "git diff $(git merge-base HEAD %s)..HEAD --unified=0" - (or base "@{upstream}"))) - (t - (user-error "Unknown coverage scope: %s" scope))))) - (cj/--coverage-parse-diff-output (shell-command-to-string cmd)))) + (let ((output + (pcase scope + ('working-tree + (cj/--coverage-git-diff "HEAD")) + ('staged + (cj/--coverage-git-diff "--cached")) + ('branch-vs-main + (cj/--coverage-git-diff + (format "%s..HEAD" (cj/--coverage-git-merge-base "main")))) + ('branch-vs-parent + (cj/--coverage-git-diff + (format "%s..HEAD" + (cj/--coverage-git-merge-base (or base "@{upstream}"))))) + (_ + (user-error "Unknown coverage scope: %s" scope))))) + (cj/--coverage-parse-diff-output output))) (defun cj/--coverage-hash-keys-sorted (table) "Return a sorted list of TABLE's integer keys." |
