From 8c68b28dc21ecaab5648904d17dec37f4ee45547 Mon Sep 17 00:00:00 2001 From: Michael Wesemann Date: Sun, 26 Jul 2026 16:53:54 +0200 Subject: [PATCH] Format `list` as an aligned table The name is what the eye looks for, but it came last, behind a ragged date column, so nothing lined up. Worse, colorRepoLine rebuilt the line with strings.Fields and single spaces, which destroyed the alignment ls had produced -- the output was aligned only when colour was off. The listing line is now parsed properly instead of being split at the size field: name, date and size come out as fields, the date is re-padded to a fixed twelve columns so "Sep 28 2016" and "Jan 3 14:32" agree, and the name leads in a column sized to the longest entry. Colour decorates that layout without changing it, which a test now checks by stripping the escapes and comparing. `list -a` shows archive sizes, which were parsed and thrown away before. An empty result says so instead of printing nothing, which was indistinguishable from a failure, and the count line matches the rest of mgsh. The pattern now filters on the repository name rather than the whole listing line: matching the owner or the date was never intended and `list 2016` quietly did it. Co-Authored-By: Claude Opus 5 --- README.md | 19 +++++++- colors.go | 53 ++++++++++++++++----- commands.go | 68 ++++++++++++++++++++------- mgsh_test.go | 129 +++++++++++++++++++++++++++++++++++++-------------- version.txt | 2 +- 5 files changed, 205 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 5ee8701..9d24fcd 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ Run `help` for the full list. Highlights: | `log` | show the project log | | `edit [n]` | interactive rebase of the last n commits | | `clone [-a] ` | clone a repository (or archive) from the server | -| `list [-a] [pattern]` | list repositories on the server | +| `list [-a] [pattern]` | list repositories on the server (`-a`: archives, with sizes) | | `show ` | show a repository log directly on the server | | `archive [comment]` | snapshot the server-side repo into `./archive` | | `init` | make a new repository from the current directory | @@ -126,6 +126,23 @@ pushremote targets (in push order): Tokens are masked, so the output is safe to paste into a bug report. `config -k` prints just the setting names, one per line. +### Listing the server + +`list` shows what is on the git server, name first and aligned, ordered by +modification time — `push` touches the bare repository, so the most recently +worked-on project sits closest to the prompt: + +``` +< src > list + Betaflight3.0.0 Sep 28 2016 + website Mar 3 2024 + notes Jan 3 14:32 +3 repositories +``` + +`list -a` lists the archives instead, with their sizes; a pattern filters by +name (`list note`). + ### Overview `overview` (or `status -a`) is the one view that needs mgsh: it is the only diff --git a/colors.go b/colors.go index 2dd51ba..efbe01b 100644 --- a/colors.go +++ b/colors.go @@ -2,6 +2,7 @@ package main import ( "fmt" + "strconv" "strings" ) @@ -43,17 +44,47 @@ func padRight(s string, n int) string { return s } -// colorRepoLine colors a `list` entry: the leading `ls -ltr` date (3 fields) in -// yellow and the repository name in green. -func colorRepoLine(s string) string { - if !useColor { - return s +// formatRepoList renders the server listing for `list`: the name first, in a +// column wide enough for the longest one, then the date, and for archives the +// size. Names come first because that is what the eye scans for; putting the +// ragged date there instead is what made the old output hard to read. +// +// The order is left as it arrives: `ls -ltr` sorts by modification time, and +// `push` touches the bare repository, so the most recently worked-on project +// ends up closest to the prompt. +func formatRepoList(entries []lsEntry, withSize bool) string { + width := 0 + for _, e := range entries { + if len(e.name) > width { + width = len(e.name) + } } - parts := strings.Fields(s) - if len(parts) >= 4 { - date := strings.Join(parts[:3], " ") - name := strings.Join(parts[3:], " ") - return col(cYellow, date) + " " + col(cGreen, name) + var b strings.Builder + for _, e := range entries { + fmt.Fprintf(&b, " %s %s", col(cGreen, padRight(e.name, width)), col(cYellow, e.date)) + if withSize { + fmt.Fprintf(&b, " %s", col(cGray, fmt.Sprintf("%7s", humanSize(e.size)))) + } + b.WriteByte('\n') } - return col(cGreen, s) + return b.String() +} + +// humanSize renders a byte count compactly, the way `ls -h` does: a decimal +// only while it still carries information, so "3.2M" but "512K". +func humanSize(n int64) string { + const unit = 1024 + if n < unit { + return strconv.FormatInt(n, 10) + "B" + } + div, exp := int64(unit), 0 + for v := n / unit; v >= unit && exp < 4; v /= unit { + div *= unit + exp++ + } + v := float64(n) / float64(div) + if v < 10 { + return fmt.Sprintf("%.1f%c", v, "KMGTP"[exp]) + } + return fmt.Sprintf("%.0f%c", v, "KMGTP"[exp]) } diff --git a/commands.go b/commands.go index 2bf1830..a0c0651 100644 --- a/commands.go +++ b/commands.go @@ -14,32 +14,46 @@ import ( var ( optRe = regexp.MustCompile(`^-(\w)$`) numRe = regexp.MustCompile(`^\d+$`) - // a `ls -ltr` long-listing line: mode, link count, owner, group, size, then - // the date columns and the name. Owner and group are matched as opaque + // a `ls -ltr` long-listing line: mode, link count, owner, group, size, the + // three date columns, then the name. Owner and group are matched as opaque // fields — the bare repositories need not belong to a user or group // literally named "git". - lsEntryRe = regexp.MustCompile(`^\S+\s+\d+\s+\S+\s+\S+\s+\d+\s+(.*)$`) + lsEntryRe = regexp.MustCompile(`^\S+\s+\d+\s+\S+\s+\S+\s+(\d+)\s+(\S+)\s+(\S+)\s+(\S+)\s+(.*)$`) gitDirRe = regexp.MustCompile(`^(.*)\.git$`) sanRe = regexp.MustCompile(`[,;:\\/='"|?><-]+`) wsRe = regexp.MustCompile(`\s+`) ) -// lsEntry extracts the " " tail of a `ls -ltr` line whose -// entry name ends in suffix, with the suffix removed. It returns "" for any -// other line (the leading "total" line, entries of a different kind). -func lsEntry(line, suffix string) string { +// lsEntry is one parsed entry of the server's listing. +type lsEntry struct { + name string // with the ".git" / ".git.tar.gz" suffix removed + date string // the ls date columns, normalised to a fixed 12 columns + size int64 +} + +// parseLsEntry reads one `ls -ltr` line whose entry name ends in suffix. It +// returns false for anything else: the leading "total" line, entries of another +// kind, or output that does not look like a long listing at all. +func parseLsEntry(line, suffix string) (lsEntry, bool) { m := lsEntryRe.FindStringSubmatch(strings.TrimSpace(line)) if m == nil { - return "" + return lsEntry{}, false } - name := m[1] + name := m[5] if i := strings.Index(name, " -> "); i >= 0 { name = name[:i] // a symlinked bare repo lists as "link.git -> target.git" } if !strings.HasSuffix(name, suffix) { - return "" + return lsEntry{}, false } - return strings.TrimSuffix(name, suffix) + size, _ := strconv.ParseInt(m[1], 10, 64) + return lsEntry{ + name: strings.TrimSuffix(name, suffix), + // ls pads these itself, but only in its own column widths; re-pad so + // "Sep 28 2016" and "Jan 3 14:32" line up at 12 either way + date: fmt.Sprintf("%s %2s %5s", m[2], m[3], m[4]), + size: size, + }, true } // validProject reports whether name is usable as a project name: a single path @@ -184,20 +198,40 @@ func runCommandDepth(line string, depth int) bool { if opt["a"] { path, suffix = "./archive", ".git.tar.gz" } - pat := word(words, 1) + one, many := "repository", "repositories" + if opt["a"] { + one, many = "archive", "archives" + } + pat := strings.ToLower(word(words, 1)) lines, err := sshOut("/bin/ls -ltr " + shq(path)) if err != nil { - errorln("could not list repositories on the git server") + errorln("could not list " + many + " on the git server") break } + var entries []lsEntry for _, ln := range lines { - if pat != "" && !strings.Contains(strings.ToLower(ln), strings.ToLower(pat)) { + e, ok := parseLsEntry(ln, suffix) + // the pattern filters the name, not the whole listing line — an + // accidental match on the date or the owner helps nobody + if !ok || (pat != "" && !strings.Contains(strings.ToLower(e.name), pat)) { continue } - if name := lsEntry(ln, suffix); name != "" { - fmt.Println(colorRepoLine(name)) - } + entries = append(entries, e) } + if len(entries) == 0 { + what := "no " + many + " on the git server" + if pat != "" { + what = "no " + many + " matching '" + word(words, 1) + "'" + } + fmt.Println(col(cGray, what)) + break + } + fmt.Print(formatRepoList(entries, opt["a"])) + label := many + if len(entries) == 1 { + label = one + } + fmt.Println(col(cGray, fmt.Sprintf("%d %s", len(entries), label))) case "show": // show a repository's log directly on the server prj := PRJ diff --git a/mgsh_test.go b/mgsh_test.go index 462155c..52bef38 100644 --- a/mgsh_test.go +++ b/mgsh_test.go @@ -74,19 +74,65 @@ func TestFormatLogRecentCompact(t *testing.T) { } } -func TestColorRepoLine(t *testing.T) { +func TestFormatRepoList(t *testing.T) { useColor = false - in := "Sep 28 2016 Betaflight3.0.0" - if got := colorRepoLine(in); got != in { - t.Errorf("colorRepoLine with color off changed input: %q", got) + entries := []lsEntry{ + {name: "short", date: "Sep 28 2016", size: 4096}, + {name: "a-much-longer-name", date: "Jan 3 14:32", size: 1536}, } - useColor = true - got := colorRepoLine(in) - if !strings.Contains(got, "Betaflight3.0.0") || !strings.Contains(got, cGreen) || !strings.Contains(got, cYellow) { - t.Errorf("colorRepoLine did not color parts: %q", got) + out := formatRepoList(entries, false) + lines := strings.Split(strings.TrimRight(out, "\n"), "\n") + if len(lines) != 2 { + t.Fatalf("expected 2 lines, got %d: %q", len(lines), out) } + // order is preserved: `ls -ltr` already sorted by modification time + if !strings.Contains(lines[0], "short") || !strings.Contains(lines[1], "a-much-longer-name") { + t.Errorf("order not preserved: %q", out) + } + // the date starts at the same column on every line + if strings.Index(lines[0], "Sep") != strings.Index(lines[1], "Jan") { + t.Errorf("date column not aligned:\n%s", out) + } + if strings.Contains(out, "4.0K") { + t.Errorf("size shown for repositories: %q", out) + } + + if withSize := formatRepoList(entries, true); !strings.Contains(withSize, "4.0K") || + !strings.Contains(withSize, "1.5K") { + t.Errorf("archive sizes missing: %q", withSize) + } + + // colour must decorate the layout, never change it + useColor = true + colored := formatRepoList(entries, false) useColor = false + strip := func(s string) string { + for _, c := range []string{cReset, cGreen, cYellow, cGray} { + s = strings.ReplaceAll(s, c, "") + } + return s + } + if strip(colored) != out { + t.Errorf("colour changed the layout:\n%q\n%q", strip(colored), out) + } +} + +func TestHumanSize(t *testing.T) { + cases := []struct { + n int64 + want string + }{ + {0, "0B"}, {512, "512B"}, {1024, "1.0K"}, {1536, "1.5K"}, + {1024 * 1024, "1.0M"}, {3 * 1024 * 1024 * 1024, "3.0G"}, + // past 10 the decimal carries nothing, as with `ls -h` + {512 * 1024, "512K"}, {99 * 1024 * 1024, "99M"}, + } + for _, c := range cases { + if got := humanSize(c.n); got != c.want { + t.Errorf("humanSize(%d) = %q, want %q", c.n, got, c.want) + } + } } func TestParseConfig(t *testing.T) { @@ -147,38 +193,49 @@ gitemail = # value is only a comment } } -func TestLsEntry(t *testing.T) { - cases := []struct{ line, suffix, want string }{ - // ownership is not assumed: any user/group must list - {"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"}, - {"drwxr-xr-x 7 deploy deploy 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"}, - {"drwxr-xr-x 7 mike staff 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"}, - {"drwxr-xr-x. 7 git users 4096 Sep 28 2016 myproj.git", ".git", "Sep 28 2016 myproj"}, - // archives only match the archive suffix, and vice versa - {"-rw-r--r-- 1 git git 512 Sep 28 2016 myproj.git.tar.gz", ".git.tar.gz", "Sep 28 2016 myproj"}, - {"-rw-r--r-- 1 git git 512 Sep 28 2016 myproj.git.tar.gz", ".git", ""}, - {"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git.tar.gz", ""}, +func TestParseLsEntry(t *testing.T) { + cases := []struct { + line, suffix string + name, date string + size int64 + ok bool + }{ + // ownership is not assumed: any user/group must parse + {"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git", "myproj", "Sep 28 2016", 4096, true}, + {"drwxr-xr-x 7 deploy deploy 4096 Sep 28 2016 myproj.git", ".git", "myproj", "Sep 28 2016", 4096, true}, + {"drwxr-xr-x. 7 git users 4096 Sep 28 2016 myproj.git", ".git", "myproj", "Sep 28 2016", 4096, true}, + // a recent entry carries a time instead of a year, and still lines up + {"drwxr-xr-x 7 mike staff 224 Jan 3 14:32 myproj.git", ".git", "myproj", "Jan 3 14:32", 224, true}, + // a symlinked bare repo lists its target too — only the link name counts + {"lrwxrwxrwx 1 git git 14 Sep 28 2016 myproj.git -> /srv/other.git", ".git", "myproj", "Sep 28 2016", 14, true}, + // archives carry a size worth showing + {"-rw-r--r-- 1 git git 524288 Sep 28 2016 myproj.git.tar.gz", ".git.tar.gz", "myproj", "Sep 28 2016", 524288, true}, + // suffixes must not cross over + {"-rw-r--r-- 1 git git 512 Sep 28 2016 myproj.git.tar.gz", ".git", "", "", 0, false}, + {"drwxr-xr-x 7 git git 4096 Sep 28 2016 myproj.git", ".git.tar.gz", "", "", 0, false}, + {"lrwxrwxrwx 1 git git 5 Sep 28 2016 notes -> x.git", ".git", "", "", 0, false}, // non-entries - {"total 48", ".git", ""}, - {"", ".git", ""}, - {"drwxr-xr-x 7 git git 4096 Sep 28 2016 notes", ".git", ""}, + {"total 48", ".git", "", "", 0, false}, + {"", ".git", "", "", 0, false}, + {"drwxr-xr-x 7 git git 4096 Sep 28 2016 notes", ".git", "", "", 0, false}, } for _, c := range cases { - if got := lsEntry(c.line, c.suffix); got != c.want { - t.Errorf("lsEntry(%q, %q) = %q, want %q", c.line, c.suffix, got, c.want) + e, ok := parseLsEntry(c.line, c.suffix) + if ok != c.ok { + t.Errorf("parseLsEntry(%q, %q) ok = %v, want %v", c.line, c.suffix, ok, c.ok) + continue + } + if !ok { + continue + } + if e.name != c.name || e.size != c.size { + t.Errorf("parseLsEntry(%q) = %+v, want name %q size %d", c.line, e, c.name, c.size) + } + // every date renders to the same width, whichever form ls used + if e.date != c.date || len(e.date) != 12 { + t.Errorf("parseLsEntry(%q) date = %q (len %d), want %q at 12", + c.line, e.date, len(e.date), c.date) } - } -} - -func TestLsEntrySymlink(t *testing.T) { - // a symlinked bare repo lists its target too — only the link name counts - in := "lrwxrwxrwx 1 git git 14 Sep 28 2016 myproj.git -> /srv/other.git" - if got := lsEntry(in, ".git"); got != "Sep 28 2016 myproj" { - t.Errorf("lsEntry(symlink) = %q, want %q", got, "Sep 28 2016 myproj") - } - // and a symlink to something that is not a repo must not match - if got := lsEntry("lrwxrwxrwx 1 git git 5 Sep 28 2016 notes -> x.git", ".git"); got != "" { - t.Errorf("lsEntry(non-repo symlink) = %q, want empty", got) } } diff --git a/version.txt b/version.txt index f5cd8d2..3ced8af 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -4.0.27 +4.0.29