Skip to content

Commit dbc8616

Browse files
refactor(git): align native status engine with Go conventions
Apply the project's Go skill end to end: modernize and fieldalignment rewrites, switch instead of else in the index name decoder, errors.New for static errors, restored field docs the alignment pass dropped, a log.Trace on Load, and test cleanups — the three fallback trigger tests merge into one table, and the benchmark reuses the parity test helpers through testing.TB instead of duplicating them. Also fold in the review's simplifications: drop the dead rehash counter, parse only the cache-tree root record, key the pack object cache by struct instead of a formatted string, skip the walk entirely when the flat pool runs with untracked detection off, search a single sorted-path list per platform, and turn the ahead/behind termination flag into a plain break. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6445269 commit dbc8616

10 files changed

Lines changed: 203 additions & 206 deletions

File tree

src/gitstatus/aheadbehind.go

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ const (
1414
)
1515

1616
type queuedCommit struct {
17-
hash plumbing.Hash
1817
info *commitInfo
18+
hash plumbing.Hash
1919
}
2020

2121
type commitPQ []queuedCommit
@@ -73,9 +73,8 @@ func aheadBehind(store *objectStore, ours, theirs plumbing.Hash) (int, int, erro
7373
}
7474

7575
ahead, behind := 0, 0
76-
interesting := 2
7776

78-
for queue.Len() > 0 && interesting > 0 {
77+
for queue.Len() > 0 {
7978
commit := heap.Pop(&queue).(queuedCommit)
8079
flag := flags[commit.hash]
8180

@@ -114,8 +113,8 @@ func aheadBehind(store *objectStore, ours, theirs plumbing.Hash) (int, int, erro
114113
}
115114
}
116115

117-
// Termination heuristic: once every queued commit is flagged on
118-
// both sides, nothing one-sided remains to discover.
116+
// Termination: once every queued commit is flagged on both sides,
117+
// nothing one-sided remains to discover.
119118
allShared := true
120119
for _, qc := range queue {
121120
f := flags[qc.hash]
@@ -125,7 +124,7 @@ func aheadBehind(store *objectStore, ours, theirs plumbing.Hash) (int, int, erro
125124
}
126125
}
127126
if allShared {
128-
interesting = 0
127+
break
129128
}
130129
}
131130

src/gitstatus/bench_test.go

Lines changed: 14 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,48 +1,40 @@
11
package gitstatus
22

33
import (
4-
"context"
54
"fmt"
6-
"os"
7-
"os/exec"
85
"path/filepath"
9-
"strings"
106
"testing"
11-
12-
"github.com/stretchr/testify/require"
137
)
148

159
// BenchmarkLoad runs Load against a repo with ~1k committed files, a small
1610
// mix of unstaged/staged/untracked changes, and reports allocations.
1711
func BenchmarkLoad(b *testing.B) {
18-
if _, err := exec.LookPath("git"); err != nil {
19-
b.Skip("git not found on PATH")
20-
}
12+
skipIfNoGit(b)
2113

2214
dir := b.TempDir()
2315

24-
runBenchGit(b, dir, "init", "-q", "-b", "main", ".")
25-
runBenchGit(b, dir, "config", "user.email", "bench@example.com")
26-
runBenchGit(b, dir, "config", "user.name", "Bench")
16+
runGit(b, dir, "init", "-q", "-b", "main", ".")
17+
runGit(b, dir, "config", "user.email", "bench@example.com")
18+
runGit(b, dir, "config", "user.name", "Bench")
2719

2820
const fileCount = 1000
2921
for i := range fileCount {
3022
rel := filepath.Join("pkg", fmt.Sprintf("dir%d", i%20), fmt.Sprintf("file%d.txt", i))
31-
writeBenchFile(b, dir, rel, fmt.Sprintf("content %d\n", i))
23+
writeFile(b, dir, rel, fmt.Sprintf("content %d\n", i))
3224
}
33-
runBenchGit(b, dir, "add", ".")
34-
runBenchGit(b, dir, "commit", "-q", "-m", "seed")
25+
runGit(b, dir, "add", ".")
26+
runGit(b, dir, "commit", "-q", "-m", "seed")
3527

3628
// a small, realistic mix of changes on top of the committed tree
37-
writeBenchFile(b, dir, "pkg/dir0/file0.txt", "modified\n")
38-
writeBenchFile(b, dir, "untracked.txt", "u\n")
39-
writeBenchFile(b, dir, "pkg/dir1/staged-add.txt", "new\n")
40-
runBenchGit(b, dir, "add", "pkg/dir1/staged-add.txt")
29+
writeFile(b, dir, "pkg/dir0/file0.txt", "modified\n")
30+
writeFile(b, dir, "untracked.txt", "u\n")
31+
writeFile(b, dir, "pkg/dir1/staged-add.txt", "new\n")
32+
runGit(b, dir, "add", "pkg/dir1/staged-add.txt")
4133

4234
opts := Options{
43-
WorktreeGitDir: gitBenchPath(b, dir, "--git-dir"),
44-
CommonGitDir: gitBenchPath(b, dir, "--git-common-dir"),
45-
RepoRoot: gitBenchPath(b, dir, "--show-toplevel"),
35+
WorktreeGitDir: gitPath(b, dir, "--git-dir"),
36+
CommonGitDir: gitPath(b, dir, "--git-common-dir"),
37+
RepoRoot: gitPath(b, dir, "--show-toplevel"),
4638
}
4739

4840
b.ReportAllocs()
@@ -54,28 +46,3 @@ func BenchmarkLoad(b *testing.B) {
5446
}
5547
}
5648
}
57-
58-
func writeBenchFile(b *testing.B, dir, rel, content string) {
59-
b.Helper()
60-
full := filepath.Join(dir, filepath.FromSlash(rel))
61-
require.NoError(b, os.MkdirAll(filepath.Dir(full), 0o755))
62-
require.NoError(b, os.WriteFile(full, []byte(content), 0o644))
63-
}
64-
65-
func runBenchGit(b *testing.B, dir string, args ...string) {
66-
b.Helper()
67-
cmd := exec.CommandContext(context.Background(), "git", args...)
68-
cmd.Dir = dir
69-
cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1")
70-
out, err := cmd.CombinedOutput()
71-
require.NoErrorf(b, err, "git %s failed: %s", strings.Join(args, " "), out)
72-
}
73-
74-
func gitBenchPath(b *testing.B, dir, arg string) string {
75-
b.Helper()
76-
cmd := exec.CommandContext(context.Background(), "git", "rev-parse", "--path-format=absolute", arg)
77-
cmd.Dir = dir
78-
out, err := cmd.Output()
79-
require.NoError(b, err)
80-
return filepath.FromSlash(strings.TrimSpace(string(out)))
81-
}

src/gitstatus/gitstatus.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import (
1414
"os"
1515
"path/filepath"
1616
"time"
17+
18+
"github.com/jandedobbeleer/oh-my-posh/src/log"
1719
)
1820

1921
// Detached is the branch name reported when HEAD is not on a branch. It must
@@ -33,11 +35,14 @@ type Counts struct {
3335

3436
// Result is the outcome of a successful Load.
3537
type Result struct {
38+
// Hash is the full HEAD hash, or "(initial)" on an unborn branch.
39+
Hash string
40+
// Ref is the branch name, or Detached.
41+
Ref string
42+
// Upstream is "origin/main"-style, empty when not configured.
43+
Upstream string
3644
Working Counts
3745
Staging Counts
38-
Hash string // full HEAD hash, "(initial)" when unborn
39-
Ref string // branch name, or Detached
40-
Upstream string // "origin/main"-style, "" when not configured
4146
Ahead int
4247
Behind int
4348
UpstreamGone bool
@@ -55,6 +60,8 @@ type Options struct {
5560
// Load computes the working tree and staging area status for the repository
5661
// described by opts. Any error means the caller must fall back to exec git.
5762
func Load(opts Options) (*Result, error) {
63+
defer log.Trace(time.Now(), opts.RepoRoot)
64+
5865
untrackedMode := opts.UntrackedMode
5966
if untrackedMode == "" {
6067
untrackedMode = "normal"

src/gitstatus/gitstatus_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ func setupIntentToAdd(t *testing.T, dir string) {
357357

358358
// --- test infrastructure -----------------------------------------------
359359

360-
func skipIfNoGit(t *testing.T) {
360+
func skipIfNoGit(t testing.TB) {
361361
t.Helper()
362362
if _, err := exec.LookPath("git"); err != nil {
363363
t.Skip("git not found on PATH")
@@ -368,30 +368,30 @@ func skipIfNoGit(t *testing.T) {
368368
// directory for the duration of the test, so neither the native engine nor
369369
// the real git CLI pick up the developer machine's actual global gitconfig
370370
// or excludes file.
371-
func hermeticHome(t *testing.T) {
371+
func hermeticHome(t testing.TB) {
372372
t.Helper()
373373
home := t.TempDir()
374374
t.Setenv("HOME", home)
375375
t.Setenv("USERPROFILE", home)
376376
t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
377377
}
378378

379-
func initGitRepo(t *testing.T, dir string) {
379+
func initGitRepo(t testing.TB, dir string) {
380380
t.Helper()
381381
runGit(t, dir, "init", "-q", "-b", "main", ".")
382382
runGit(t, dir, "config", "user.email", "test@example.com")
383383
runGit(t, dir, "config", "user.name", "Test")
384384
runGit(t, dir, "config", "core.autocrlf", "false")
385385
}
386386

387-
func writeFile(t *testing.T, dir, rel, content string) {
387+
func writeFile(t testing.TB, dir, rel, content string) {
388388
t.Helper()
389389
full := filepath.Join(dir, filepath.FromSlash(rel))
390390
require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755))
391391
require.NoError(t, os.WriteFile(full, []byte(content), 0o644))
392392
}
393393

394-
func runGit(t *testing.T, dir string, args ...string) string {
394+
func runGit(t testing.TB, dir string, args ...string) string {
395395
t.Helper()
396396
out, err := gitCommand(dir, args...)
397397
require.NoErrorf(t, err, "git %s failed: %s", strings.Join(args, " "), out)
@@ -400,7 +400,7 @@ func runGit(t *testing.T, dir string, args ...string) string {
400400

401401
// runGitAllowFail runs git and returns its output even on a non-zero exit,
402402
// for commands like `git merge` that legitimately fail on conflict.
403-
func runGitAllowFail(t *testing.T, dir string, args ...string) string {
403+
func runGitAllowFail(t testing.TB, dir string, args ...string) string {
404404
t.Helper()
405405
out, _ := gitCommand(dir, args...)
406406
return out
@@ -414,7 +414,7 @@ func gitCommand(dir string, args ...string) (string, error) {
414414
return string(out), err
415415
}
416416

417-
func gitPath(t *testing.T, dir, arg string) string {
417+
func gitPath(t testing.TB, dir, arg string) string {
418418
t.Helper()
419419
out := runGit(t, dir, "rev-parse", "--path-format=absolute", arg)
420420
return filepath.FromSlash(strings.TrimSpace(out))

src/gitstatus/index.go

Lines changed: 50 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type indexEntry struct {
4848
IntentToAdd bool
4949
}
5050

51-
// cacheTree is one entry of the TREE extension. Entries < 0 marks the span
51+
// cacheTree is the root record of the TREE extension. Entries < 0 marks it
5252
// invalidated.
5353
type cacheTree struct {
5454
Path string
@@ -57,8 +57,11 @@ type cacheTree struct {
5757
}
5858

5959
type gitIndex struct {
60-
Entries []indexEntry
61-
CacheTree []cacheTree
60+
// CacheTreeRoot holds the first TREE extension record (the repository
61+
// root); the staging fast path needs nothing else, so subtree records
62+
// are never parsed. Nil when the extension is absent.
63+
CacheTreeRoot *cacheTree
64+
Entries []indexEntry
6265
}
6366

6467
// decodeIndex parses an index file. It fails on unsupported versions and on
@@ -119,16 +122,16 @@ func decodeEntries(data []byte, version, count uint32, idx *gitIndex) ([]byte, e
119122

120123
var name []byte
121124
var err error
122-
if version == 4 {
125+
126+
switch version {
127+
case 4:
123128
name, data, err = decodeNameV4(data[consumed:], prevName)
124-
if err != nil {
125-
return nil, err
126-
}
127-
} else {
129+
default:
128130
name, data, err = decodeNamePadded(data, consumed, nameLen)
129-
if err != nil {
130-
return nil, err
131-
}
131+
}
132+
133+
if err != nil {
134+
return nil, err
132135
}
133136

134137
prevName = name
@@ -257,48 +260,51 @@ func decodeExtensions(data []byte, idx *gitIndex) error {
257260
return nil
258261
}
259262

260-
// decodeTreeExtension parses the cache-tree extension: a sequence of
261-
// "path NUL entry-count SP subtree-count LF [hash]" records, hash present
262-
// only for valid (non-negative entry-count) records.
263+
// decodeTreeExtension parses only the first record of the cache-tree
264+
// extension — "path NUL entry-count SP subtree-count LF [hash]", hash
265+
// present only when the entry-count is non-negative. The first record is
266+
// the repository root, the only one the staging fast path consumes; the
267+
// remaining subtree records are skipped wholesale (the extension block is
268+
// length-delimited, so nothing after it depends on parsing them).
263269
func decodeTreeExtension(data []byte, idx *gitIndex) error {
264-
for len(data) > 0 {
265-
nul := -1
266-
for i := range data {
267-
if data[i] == 0 {
268-
nul = i
269-
break
270-
}
271-
}
272-
if nul < 0 {
273-
return errIndexMalformed
270+
if len(data) == 0 {
271+
return nil
272+
}
273+
274+
nul := -1
275+
for i := range data {
276+
if data[i] == 0 {
277+
nul = i
278+
break
274279
}
280+
}
281+
if nul < 0 {
282+
return errIndexMalformed
283+
}
275284

276-
entry := cacheTree{Path: string(data[:nul])}
277-
data = data[nul+1:]
285+
root := cacheTree{Path: string(data[:nul])}
286+
data = data[nul+1:]
278287

279-
count, rest, err := readASCIIInt(data, ' ')
280-
if err != nil {
281-
return err
282-
}
283-
entry.Entries = count
288+
count, rest, err := readASCIIInt(data, ' ')
289+
if err != nil {
290+
return err
291+
}
292+
root.Entries = count
284293

285-
_, rest, err = readASCIIInt(rest, '\n')
286-
if err != nil {
287-
return err
288-
}
289-
data = rest
294+
_, rest, err = readASCIIInt(rest, '\n')
295+
if err != nil {
296+
return err
297+
}
298+
data = rest
290299

291-
if count >= 0 {
292-
if len(data) < 20 {
293-
return errIndexMalformed
294-
}
295-
copy(entry.Hash[:], data[:20])
296-
data = data[20:]
300+
if count >= 0 {
301+
if len(data) < 20 {
302+
return errIndexMalformed
297303
}
298-
299-
idx.CacheTree = append(idx.CacheTree, entry)
304+
copy(root.Hash[:], data[:20])
300305
}
301306

307+
idx.CacheTreeRoot = &root
302308
return nil
303309
}
304310

src/gitstatus/objects.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ import (
1111

1212
// commitInfo is the subset of a commit object the status engine needs.
1313
type commitInfo struct {
14+
Parents []plumbing.Hash
15+
// CommitterWhen is the committer timestamp in unix seconds.
16+
CommitterWhen int64
1417
Tree plumbing.Hash
15-
Parents []plumbing.Hash
16-
CommitterWhen int64 // unix seconds
1718
}
1819

1920
func readCommit(store *objectStore, h plumbing.Hash) (*commitInfo, error) {

0 commit comments

Comments
 (0)