исправлены находки ревью проектной стороны

- манифест читается так, как записан: решётка внутри строки не открывает
  комментарий, скобка внутри комментария не закрывает массив, имя внутри
  комментария не становится подпиской; новый ключ встаёт после массива,
  а не внутрь него
- всё записываемое проходит через manifest.Quote — обратный слэш в пути
  делал файл, который инструмент сам не читает
- маркер локальной части переехал в doc и пропускает огороженные блоки:
  процитированный в примере маркер больше не считается границей, а копия
  без маркера не перезаписывается молча
- лишний позиционный аргумент отсекается: flag прекращал разбор и прятал
  флаги после себя, из-за чего pull, list и check игнорировали --for
- заведены тесты проверок копий, включая молчание на исправной копии
This commit is contained in:
av
2026-07-27 21:16:29 +03:00
parent 23d88c4048
commit b6b0976c19
19 changed files with 904 additions and 197 deletions
+6 -10
View File
@@ -41,12 +41,9 @@ func runAdd(env Env, args []string) ExitCode {
}
topic = *topicFlag
}
if rest := fs.Args(); len(rest) > 0 {
if topic != "" && topic != rest[0] {
fmt.Fprintf(env.Err, "the topic is named twice and differently: %q and %q\n", topic, rest[0])
return Usage
}
topic = rest[0]
if left := fs.Args(); len(left) > 0 {
fmt.Fprintf(env.Err, "convy add takes one topic, and %q came after it as well\n", left[0])
return Usage
}
o, code := openProject(env, *root)
@@ -70,12 +67,11 @@ func runAdd(env Env, args []string) ExitCode {
fmt.Fprintln(env.Err, "convy add without arguments asks which topic, and there is no terminal to ask on; name the topic as an argument")
return Usage
}
answer, err := newDialogue(env).ask(topicField(o, c))
if err != nil {
fmt.Fprintln(env.Err, "\ninterrupted, nothing was written")
given := map[string]string{}
if err := askAll(env, []Field{topicField(o, c)}, given); err != nil {
return Usage
}
topic = answer
topic = given["topic"]
}
if !o.Suite.Manifest.TopicLive(topic) {
+32 -19
View File
@@ -11,7 +11,6 @@ import (
"git.vakhrushev.me/av/convy/internal/check"
"git.vakhrushev.me/av/convy/internal/doc"
"git.vakhrushev.me/av/convy/internal/manifest"
"git.vakhrushev.me/av/convy/internal/project"
)
@@ -29,13 +28,19 @@ func runCheck(env Env, args []string) ExitCode {
if err := fs.Parse(args); err != nil {
return Usage
}
if code := noStrayArgs(env, "convy check", fs.Args()); code != OK {
return code
}
dir, code := projectRoot(env, *root)
if code != OK {
return code
}
m, err := manifest.LoadProject(dir)
if err != nil {
m, code := loadProject(env, dir)
if code != OK {
return code
}
if err := distinctDirs(m); err != nil {
fmt.Fprintln(env.Err, err)
return Usage
}
@@ -47,10 +52,16 @@ func runCheck(env Env, args []string) ExitCode {
var docs []*doc.Document
var broken []error
for _, name := range names {
found, errs := copies(dir, m.Components[name].Dir)
c := m.Components[name]
if c.Dir == "" {
fmt.Fprintf(env.Err, "the component %q names no dir, and there is nothing to look in\n", name)
return Usage
}
found, errs := copies(dir, c.Dir)
docs = append(docs, found...)
broken = append(broken, errs...)
}
docs = distinct(docs)
rep := check.Copies(docs)
for _, err := range broken {
@@ -104,23 +115,25 @@ func copies(root, dir string) ([]*doc.Document, []error) {
return docs, broken
}
// distinct drops a document reached through two components. Directories are
// checked for equality before this, but one may still lie inside another, and a
// finding printed twice reads as two.
func distinct(docs []*doc.Document) []*doc.Document {
seen := make(map[string]bool, len(docs))
out := docs[:0]
for _, d := range docs {
if seen[d.Path] {
continue
}
seen[d.Path] = true
out = append(out, d)
}
return out
}
func printCopyReport(w io.Writer, rep *check.Report, files, comps int, quiet bool) {
findings := rep.Findings()
current := ""
for _, f := range findings {
if f.Path != current {
if current != "" {
fmt.Fprintln(w)
}
fmt.Fprintf(w, "%s\n", f.Path)
current = f.Path
}
where := ""
if f.Line > 0 {
where = fmt.Sprintf(":%d", f.Line)
}
fmt.Fprintf(w, " %s%s %s [%s]\n", f.Severity, where, f.Msg, f.Family)
}
printFindings(w, findings)
if quiet {
return
+26
View File
@@ -11,6 +11,7 @@ import (
"fmt"
"io"
"os"
"strings"
)
// ExitCode is the exit status of the process.
@@ -118,6 +119,31 @@ everything at once and ask nothing — that mode is for agents and scripts.
`)
}
// noStrayArgs turns down an argument the command has no place for. The flag
// package stops parsing at the first argument that is not a flag, so a stray one
// does not merely sit there unused — it hides every flag written after it, and
// the command then does something other than what was asked in silence.
func noStrayArgs(env Env, name string, rest []string) ExitCode {
if len(rest) == 0 {
return OK
}
fmt.Fprintf(env.Err, "%s takes no argument, and %q was given; a component is named by --for\n", name, rest[0])
return Usage
}
// split reads a comma-separated list off the command line. An axis of a
// component is a list — a component may sit on two stacks at once — and one
// flag repeated is worse to type than one flag with commas in it.
func split(value string) []string {
var out []string
for _, part := range strings.Split(value, ",") {
if part = strings.TrimSpace(part); part != "" {
out = append(out, part)
}
}
return out
}
// Main is the entry point of the process.
func Main() int {
dir, err := os.Getwd()
+8 -8
View File
@@ -28,7 +28,7 @@ const projectSkeleton = `# What this repository takes from a conventions suite.
# Where the copies come from: a path on disk, relative to this file or absolute,
# or a git repository over http or https. A trailing #branch, #tag or #commit
# pins a revision.
source = "%s"
source = %s
# ─── Components ─────────────────────────────────────────────────────────────
#
@@ -54,6 +54,9 @@ func runInit(env Env, args []string) ExitCode {
if err := fs.Parse(args); err != nil {
return Usage
}
if code := noStrayArgs(env, "convy init", fs.Args()); code != OK {
return code
}
where := *root
if where == "" {
@@ -102,7 +105,7 @@ func runInit(env Env, args []string) ExitCode {
return Failed
}
entries := [][2]string{{"dir", quote(given["dir"])}}
entries := [][2]string{{"dir", manifest.Quote(given["dir"])}}
if list := split(given["lang"]); len(list) > 0 {
entries = append(entries, [2]string{"lang", array(list)})
}
@@ -111,7 +114,7 @@ func runInit(env Env, args []string) ExitCode {
}
entries = append(entries, [2]string{"topics", "[]"})
body, err := manifest.AddTable([]byte(fmt.Sprintf(projectSkeleton, given["source"])),
body, err := manifest.AddTable(fmt.Appendf(nil, projectSkeleton, manifest.Quote(given["source"])),
"components."+given["component"], entries)
if err != nil {
fmt.Fprintln(env.Err, err)
@@ -179,14 +182,11 @@ func initFields() []Field {
}}
}
func quote(s string) string {
return `"` + strings.ReplaceAll(s, `"`, `\"`) + `"`
}
// array writes a TOML array of strings.
func array(values []string) string {
parts := make([]string, len(values))
for i, v := range values {
parts[i] = quote(v)
parts[i] = manifest.Quote(v)
}
return "[" + strings.Join(parts, ", ") + "]"
}
+3
View File
@@ -23,6 +23,9 @@ func runList(env Env, args []string) ExitCode {
if err := fs.Parse(args); err != nil {
return Usage
}
if code := noStrayArgs(env, "convy list", fs.Args()); code != OK {
return code
}
o, code := openProject(env, *root)
if code != OK {
+21 -8
View File
@@ -4,6 +4,7 @@ import (
"fmt"
"os"
"path/filepath"
"strings"
"git.vakhrushev.me/av/convy/internal/manifest"
"git.vakhrushev.me/av/convy/internal/source"
@@ -40,13 +41,9 @@ func openProject(env Env, root string) (*opened, ExitCode) {
if code != OK {
return nil, code
}
m, err := manifest.LoadProject(dir)
if err != nil {
fmt.Fprintln(env.Err, err)
return nil, Usage
}
for _, key := range m.Undecoded {
fmt.Fprintf(env.Err, "warning: %s: the key %s is unknown to the tool\n", m.Path, key)
m, code := loadProject(env, dir)
if code != OK {
return nil, code
}
if m.Source == "" {
fmt.Fprintf(env.Err, "%s names no source: a copy comes from a suite, and the manifest is where the suite is named\n", m.Path)
@@ -93,6 +90,22 @@ func openProject(env Env, root string) (*opened, ExitCode) {
return o, OK
}
// loadProject reads the project manifest and says what it did not understand.
// A typo in a key costs a whole component: `dyr` instead of `dir` leaves the
// component pointing at the root of the repository, and nothing else would say
// so.
func loadProject(env Env, dir string) (*manifest.Project, ExitCode) {
m, err := manifest.LoadProject(dir)
if err != nil {
fmt.Fprintln(env.Err, err)
return nil, Usage
}
for _, key := range m.Undecoded {
fmt.Fprintf(env.Err, "warning: %s: the key %s is unknown to the tool\n", m.Path, key)
}
return m, OK
}
// projectRoot finds the manifest of the project. A project command typed inside
// a suite does not do anything at a guess: it says where it is and names the
// command of that level.
@@ -133,7 +146,7 @@ func componentOf(env Env, m *manifest.Project, name string) (string, manifest.Co
func components(env Env, m *manifest.Project, name string) ([]string, ExitCode) {
if name != "" {
if _, ok := m.Components[name]; !ok {
fmt.Fprintf(env.Err, "the project declares no component %q\n", name)
fmt.Fprintf(env.Err, "the project declares no component %q; it declares: %s\n", name, strings.Join(m.Names(), ", "))
return nil, Usage
}
return []string{name}, OK
+140
View File
@@ -1,12 +1,16 @@
package cli_test
import (
"fmt"
"os"
"path"
"path/filepath"
"strings"
"testing"
"git.vakhrushev.me/av/convy/internal/check"
"git.vakhrushev.me/av/convy/internal/cli"
"git.vakhrushev.me/av/convy/internal/doc"
)
// readingGuide stands in for what a suite puts next to its copies: the short
@@ -343,6 +347,11 @@ func TestTheLanguageMayLiveApartFromTheSuite(t *testing.T) {
}
checkClean(t, suiteRoot)
// A check that did not run says so: silence would read as a check passed.
if _, out := run(t, suiteRoot, "", false, "suite", "check"); !strings.Contains(out, "went unchecked") {
t.Errorf("the check did not say it left the language alone:\n%s", out)
}
root := wired(t, suiteRoot, "--component", "backend", "--dir", "docs/conventions", "--lang", "go")
if code, out := run(t, root, "", false, "add", "time"); code != cli.OK {
t.Fatalf("convy add returned %d: %s", code, out)
@@ -385,3 +394,134 @@ func TestCheckCatchesALocalRuleAboveTheMarker(t *testing.T) {
t.Errorf("the finding does not say what is at stake:\n%s", out)
}
}
// copyClean is the project half of the invariant worth the most: what the tool
// writes, the tool accepts — and accepts in silence, warnings included.
func copyClean(t *testing.T, root, dir string) {
t.Helper()
var docs []*doc.Document
entries, err := os.ReadDir(filepath.Join(root, filepath.FromSlash(dir)))
if err != nil {
t.Fatal(err)
}
for _, e := range entries {
if filepath.Ext(e.Name()) != ".md" {
continue
}
rel := path.Join(dir, e.Name())
d, err := doc.Load(rel, filepath.Join(root, filepath.FromSlash(rel)))
if err != nil {
t.Fatalf("loading %s: %v", rel, err)
}
if d.Front.Origin != "" {
docs = append(docs, d)
}
}
if len(docs) == 0 {
t.Fatalf("no copy was assembled in %s", dir)
}
rep := check.Copies(docs)
if len(rep.Findings()) == 0 {
return
}
var b strings.Builder
for _, f := range rep.Findings() {
fmt.Fprintf(&b, " %s: %s\n", f.Path, f.Msg)
}
t.Fatalf("the copies the tool assembled do not check clean:\n%s", b.String())
}
func TestWhatTheToolAssemblesTheToolAccepts(t *testing.T) {
suiteRoot := subscribable(t)
root := wired(t, suiteRoot, "--component", "backend", "--dir", "docs/conventions", "--lang", "go")
run(t, root, "", false, "add", "time")
run(t, root, "", false, "add", "logging")
copyClean(t, root, "docs/conventions")
}
// The flag package stops at the first argument that is not a flag, so a stray
// one hides every flag written after it and the command quietly does something
// else than what was asked.
func TestProjectCommandsTurnDownAStrayArgument(t *testing.T) {
suiteRoot := subscribable(t)
root := wired(t, suiteRoot, "--component", "backend", "--dir", "docs/conventions", "--lang", "go")
for _, args := range [][]string{
{"pull", "backend"},
{"list", "backend"},
{"check", "backend"},
{"init", "somewhere"},
} {
t.Run(args[0], func(t *testing.T) {
code, out := run(t, root, "", false, args...)
if code == cli.OK {
t.Fatalf("the stray argument went through:\n%s", out)
}
if !strings.Contains(out, "takes no argument") {
t.Errorf("the refusal does not say what is wrong:\n%s", out)
}
})
}
}
// The marker is always left by the assembler, so a copy without one was edited
// by hand — and everything in it would count as suite text to be replaced.
func TestPullRefusesACopyWhoseMarkerIsGone(t *testing.T) {
suiteRoot := subscribable(t)
root := wired(t, suiteRoot, "--component", "backend", "--dir", "docs/conventions", "--lang", "go")
run(t, root, "", false, "add", "time")
name := filepath.Join(root, "docs", "conventions", "time.md")
body := read(t, root, "docs/conventions/time.md")
body = strings.Replace(body, "<!-- conv:local -->", "Заметка, написанная руками.", 1)
if err := os.WriteFile(name, []byte(body), 0o644); err != nil {
t.Fatal(err)
}
code, out := run(t, root, "", false, "pull")
if code == cli.OK {
t.Fatalf("the pull went ahead over a hand-edited copy:\n%s", out)
}
if !strings.Contains(out, "carries no <!-- conv:local --> marker") {
t.Errorf("the refusal does not say why:\n%s", out)
}
if got := read(t, root, "docs/conventions/time.md"); !strings.Contains(got, "Заметка, написанная руками") {
t.Errorf("the hand-written text was destroyed anyway:\n%s", got)
}
}
// Whatever the tool writes into the manifest it has to read back, and a
// backslash is the ordinary way that fails.
func TestInitWritesAManifestItCanRead(t *testing.T) {
suiteRoot := subscribable(t)
root := wired(t, suiteRoot, "--component", "backend", "--dir", `docs\conventions`)
code, out := run(t, root, "", false, "list")
if code != cli.OK {
t.Fatalf("the manifest the tool wrote does not parse: %d\n%s", code, out)
}
if !strings.Contains(out, `docs\conventions`) {
t.Errorf("the directory came back changed:\n%s", out)
}
}
// A typo in a key costs a whole component, and the parser knows about it — so
// every command that reads the manifest has to pass that on.
func TestCheckSaysWhatItDidNotUnderstand(t *testing.T) {
suiteRoot := subscribable(t)
root := wired(t, suiteRoot, "--component", "backend", "--dir", "docs/conventions", "--lang", "go")
body := read(t, root, ".conventions.toml")
body = strings.Replace(body, `dir = "docs/conventions"`, `dyr = "docs/conventions"`, 1)
if err := os.WriteFile(filepath.Join(root, ".conventions.toml"), []byte(body), 0o644); err != nil {
t.Fatal(err)
}
code, out := run(t, root, "", false, "check")
if !strings.Contains(out, "dyr") {
t.Errorf("the typo in the manifest went unmentioned:\n%s", out)
}
if code == cli.OK {
t.Errorf("a component with no dir was checked anyway:\n%s", out)
}
}
+3
View File
@@ -21,6 +21,9 @@ func runPull(env Env, args []string) ExitCode {
if err := fs.Parse(args); err != nil {
return Usage
}
if code := noStrayArgs(env, "convy pull", fs.Args()); code != OK {
return code
}
o, code := openProject(env, *root)
if code != OK {
+28 -15
View File
@@ -55,21 +55,7 @@ func runSuiteCheck(env Env, args []string) ExitCode {
func printReport(w io.Writer, rep *check.Report, s *suite.Suite, quiet bool) {
findings := rep.Findings()
current := ""
for _, f := range findings {
if f.Path != current {
if current != "" {
fmt.Fprintln(w)
}
fmt.Fprintf(w, "%s\n", f.Path)
current = f.Path
}
where := ""
if f.Line > 0 {
where = fmt.Sprintf(":%d", f.Line)
}
fmt.Fprintf(w, " %s%s %s [%s]\n", f.Severity, where, f.Msg, f.Family)
}
printFindings(w, findings)
if quiet {
return
@@ -81,6 +67,12 @@ func printReport(w io.Writer, rep *check.Report, s *suite.Suite, quiet bool) {
plural(len(s.Docs), "file"),
plural(len(s.Manifest.LiveTopics()), "topic"),
s.Manifest.Language.Version, s.Manifest.Language.Lang)
// A check that was skipped says so. Reaching the language costs a fetch and
// this check runs on every edit, so the documents about the language go
// unread — and a check silently not run reads exactly like a check passed.
if spec := s.Manifest.Language.Source; spec != "" {
fmt.Fprintf(w, "the language lies at %s and was not fetched: its documents went unchecked\n", spec)
}
switch {
case rep.Errors() > 0:
fmt.Fprintf(w, "errors: %d, warnings: %d\n", rep.Errors(), rep.Warnings())
@@ -98,3 +90,24 @@ func plural(n int, noun string) string {
}
return fmt.Sprintf("%d %ss", n, noun)
}
// printFindings lays the findings out grouped by file. Both checks print them
// the same way: a finding of the suite and a finding of a copy are read by the
// same person, and two layouts would be two things to learn.
func printFindings(w io.Writer, findings []check.Finding) {
current := ""
for _, f := range findings {
if f.Path != current {
if current != "" {
fmt.Fprintln(w)
}
fmt.Fprintf(w, "%s\n", f.Path)
current = f.Path
}
where := ""
if f.Line > 0 {
where = fmt.Sprintf(":%d", f.Line)
}
fmt.Fprintf(w, " %s%s %s [%s]\n", f.Severity, where, f.Msg, f.Family)
}
}
-14
View File
@@ -5,7 +5,6 @@ import (
"fmt"
"io"
"sort"
"strings"
"git.vakhrushev.me/av/convy/internal/doc"
"git.vakhrushev.me/av/convy/internal/lang"
@@ -142,19 +141,6 @@ func listRetired(w io.Writer, s *suite.Suite) {
fmt.Fprintln(w, "\nnone of these names is ever handed out again")
}
// split reads a comma-separated list off the command line. An axis of a
// component is a list — a component may sit on two stacks at once — and one
// flag repeated is worse to type than one flag with commas in it.
func split(value string) []string {
var out []string
for _, part := range strings.Split(value, ",") {
if part = strings.TrimSpace(part); part != "" {
out = append(out, part)
}
}
return out
}
func sortedKeys(m map[string]string) []string {
keys := make([]string, 0, len(m))
for k := range m {