diff --git a/README.md b/README.md index 60c4d53..4fdf2df 100644 --- a/README.md +++ b/README.md @@ -167,14 +167,17 @@ TUI when stdin/stdout are terminals and prints help when piped. Later: | Command | What it does | | --- | --- | -| `zn vault list` | Every saved vault and server, the default starred | +| `zn vault list` | Every saved vault and server, the default starred. Entries marked `app` come from the desktop app and are managed there | | `zn use ` | Set the terminal default (`zn use app` follows desktop again); desktop-managed scripts keep their own app default | | `zn disconnect ` | Forget a server and its token | +| `zn vault remove ` | Forget one of zn's own vaults or servers (a server's token too); note files stay | | `zn vault mode root\|inbox` | Move a vault between the flat layout and the classic `inbox/` layout | The list lives in `~/.config/zennotes/workspaces.toml`; server tokens go to `credentials.toml` next to it, readable only by you. Scripts and CI can skip -the store with `--server ` and `ZENNOTES_REMOTE_TOKEN`. +the store with `--server ` and `ZENNOTES_REMOTE_TOKEN`. If either file +stops parsing, zn says so (`zn doctor`, `zn status`) and refuses to write over +it rather than replace it with an empty list. ## Commands diff --git a/internal/cli/args.go b/internal/cli/args.go index 38c951b..d0ce40c 100644 --- a/internal/cli/args.go +++ b/internal/cli/args.go @@ -25,6 +25,16 @@ type Args struct { var shortFlagRe = regexp.MustCompile(`^-[A-Za-z][\w-]*$`) +// longFlagRe is `--` followed by a letter. `---` is not a flag, so a body +// that starts with a frontmatter fence can follow --body. +var longFlagRe = regexp.MustCompile(`^--[A-Za-z]`) + +// startsAFlag says whether the token after a value flag is the next flag +// (or the `--` terminator) rather than the value. +func startsAFlag(token string) bool { + return token == "--" || token == "-h" || longFlagRe.MatchString(token) +} + // valuelessFlags are the long flags that are switches, never `--flag `. // Without this list a switch written before a positional swallowed it: // `zn open --new-window ~/notes` parsed as new-window="~/notes" with no path, @@ -68,7 +78,7 @@ func Parse(argv []string) Args { continue } name := token[2:] - if !valuelessFlags[name] && i+1 < len(argv) && !strings.HasPrefix(argv[i+1], "--") { + if !valuelessFlags[name] && i+1 < len(argv) && !startsAFlag(argv[i+1]) { args.push(name, argv[i+1]) i++ } else { diff --git a/internal/cli/args_test.go b/internal/cli/args_test.go index 4bec861..8569ba2 100644 --- a/internal/cli/args_test.go +++ b/internal/cli/args_test.go @@ -2,6 +2,8 @@ package cli import ( "reflect" + "strconv" + "strings" "testing" ) @@ -64,3 +66,57 @@ func TestOpenLaunchArgsAndMessage(t *testing.T) { t.Fatalf("two, -n: %q", got) } } + +// A body that opens with a frontmatter fence starts with `---`, which is +// not a flag; it must be taken as the value, while a real flag or the `--` +// terminator after a value flag still means the value is missing. +func TestValueFlagsAcceptValuesThatStartWithDashes(t *testing.T) { + fm := "---\ntitle: x\n---\nbody" + name, args, err := parseCommand([]string{"write", "a.md", "--body", fm}) + if err != nil || name != "write" || args.Str("body") != fm { + t.Fatalf("frontmatter body: %q %v %v", name, args.Flags, err) + } + if _, args, err := parseCommand([]string{"capture", "--title", "---", "text"}); err != nil || args.Str("title") != "---" { + t.Fatalf("bare --- as a value: %v %v", args.Flags, err) + } + if _, args, err := parseCommand([]string{"write", "a.md", "--body", "-x"}); err != nil || args.Str("body") != "-x" { + t.Fatalf("single dash value: %v %v", args.Flags, err) + } + for _, argv := range [][]string{ + {"create", "--title", "--json"}, + {"create", "--title", "--", "x"}, + {"create", "--title", "-h"}, + {"create", "--title"}, + } { + if _, _, err := parseCommand(argv); err == nil || !strings.Contains(err.Error(), "needs a value") { + t.Errorf("%v: want a missing-value error, got %v", argv, err) + } + } + if got := Parse([]string{"--body", fm}); got.Str("body") != fm { + t.Fatalf("Parse: %v", got.Flags) + } +} + +func TestSuggestionsCountSwapsAsOneEdit(t *testing.T) { + roots := commandRoots() + for input, want := range map[string]string{"lsit": "list", "serach": "search", "craete": "create", "sevrer": "server", "lits": "list", "statsu": "status"} { + if got := suggestion(input, roots); got != " Did you mean "+strconv.Quote(want)+"?" { + t.Errorf("suggestion(%q) = %q, want %q", input, got, want) + } + } + if got := suggestion("zzzzzz", roots); got != "" { + t.Errorf("far-off input must not get a suggestion: %q", got) + } +} + +// `--tag` is the flag form of tag find's positional; it must satisfy the +// positional requirement the way --path does for note commands. +func TestTagFindAcceptsTheTagFlag(t *testing.T) { + name, args, err := parseCommand([]string{"tag", "find", "--tag", "work", "--json"}) + if err != nil || name != "tag find" || args.Str("tag") != "work" { + t.Fatalf("tag find --tag: %q %v %v", name, args.Flags, err) + } + if _, _, err := parseCommand([]string{"tag", "find"}); err == nil { + t.Fatal("tag find without a tag must be a usage error") + } +} diff --git a/internal/cli/commands_diagnostics.go b/internal/cli/commands_diagnostics.go index 9538a5e..8011bda 100644 --- a/internal/cli/commands_diagnostics.go +++ b/internal/cli/commands_diagnostics.go @@ -28,6 +28,7 @@ type workspaceStatus struct { Location string `json:"location,omitempty"` AuthConfigured bool `json:"authConfigured"` Problem string `json:"problem,omitempty"` + Warning string `json:"warning,omitempty"` InstallationOwner string `json:"installationOwner,omitempty"` } @@ -41,6 +42,9 @@ func currentStatus(args Args) (workspaceStatus, error) { if install, err := selfupdate.Current(); err == nil { st.InstallationOwner = install.Owner } + if _, err := config.LoadWorkspacesFile(); err != nil { + st.Warning = "saved vaults and servers are unavailable: " + err.Error() + } switch { case args.Str("server") != "": st.SelectedBy = "--server" @@ -91,6 +95,9 @@ func cmdStatus(args Args) error { emitLine("Installation owner: " + st.InstallationOwner) emitLine("Config: " + st.ConfigPath) emitLine("Workspace source: " + st.Source + " ยท selected by " + st.SelectedBy) + if st.Warning != "" { + emitLine("Warning: " + st.Warning) + } if st.Problem != "" { emitLine(st.Problem) } else { @@ -118,6 +125,8 @@ func cmdDoctor(ctx context.Context, args Args) error { checks = append(checks, diagnosticCheck{Name: name, OK: err == nil, Message: message}) } add("configuration", config.ConfigTomlPath(), config.ValidateConfig()) + ws, wsErr := config.LoadWorkspacesFile() + add("saved vaults", fmt.Sprintf("%d vaults and %d servers in %s", len(ws.Vaults), len(ws.Servers), config.WorkspacesPath()), wsErr) if installation, err := selfupdate.Current(); err == nil { add("installation", installation.Owner+": "+installation.Instruction, nil) } @@ -131,11 +140,11 @@ func cmdDoctor(ctx context.Context, args Args) error { add("editor", "VISUAL/EDITOR unset; config edit uses the platform fallback", nil) } if info, err := os.Stat(config.CredentialsPath()); err == nil { - var permissionErr error - if runtime.GOOS != "windows" && info.Mode().Perm()&0o077 != 0 { - permissionErr = fmt.Errorf("credentials.toml must be private (mode 0600)") + problem := config.CredentialsProblem() + if problem == nil && runtime.GOOS != "windows" && info.Mode().Perm()&0o077 != 0 { + problem = fmt.Errorf("credentials.toml must be private (mode 0600)") } - add("credentials", "Stored credentials are private", permissionErr) + add("credentials", "Stored credentials are private", problem) } target, err := ResolveTargetFromArgs(args) add("workspace", "Workspace selected", err) diff --git a/internal/cli/commands_vault_capture.go b/internal/cli/commands_vault_capture.go index f4a286d..9d208e4 100644 --- a/internal/cli/commands_vault_capture.go +++ b/internal/cli/commands_vault_capture.go @@ -65,24 +65,50 @@ func cmdVaultInfo(ctx context.Context, b backend.Backend, args Args) error { } // vaultListEntry keeps `root` for local entries so scripts reading the JSON -// keep working now that servers appear in the same list. +// keep working now that servers appear in the same list. Source says who +// saved the entry: "terminal" for zn's own list (what `zn use` and `zn vault +// remove` act on), "app" for the ZenNotes desktop app's, which zn only reads. type vaultListEntry struct { Name string `json:"name"` Kind string `json:"kind"` + Source string `json:"source"` Root string `json:"root,omitempty"` BaseURL string `json:"baseUrl,omitempty"` LastOpenedAt *int64 `json:"lastOpenedAt"` IsDefault bool `json:"isDefault"` } +const ( + sourceTerminal = "terminal" + sourceApp = "app" +) + +// desktopWorkspaces is the desktop app's vault and server list in the shape +// of zn's own, so the same name/path/URL/host matching applies to both. +func desktopWorkspaces() config.Workspaces { + ws := config.Workspaces{} + for _, v := range config.KnownVaults() { + ws.Vaults = append(ws.Vaults, config.LocalWorkspace{Name: v.Name, Root: v.Root}) + } + for _, p := range config.RemoteProfiles() { + ws.Servers = append(ws.Servers, config.ServerWorkspace{Name: p.Name, URL: p.BaseURL}) + } + return ws +} + func cmdVaultList(args Args) error { source, err := backend.ResolveWorkspaceSource(args.Str("workspace-source")) if err != nil { return err } + // Every listed name must resolve through `--vault ` in this + // source, and a desktop-managed zn never resolves zn's own names. ws := config.Workspaces{} if source == "terminal" { - ws = config.LoadWorkspaces() + var loadErr error + if ws, loadErr = config.LoadWorkspacesFile(); loadErr != nil { + emitError(fmt.Sprintf("%v; zn's own vaults and servers are not shown. `zn doctor` has details.", loadErr)) + } } defaultTarget, err := ResolveTargetFromArgs(args) if err != nil && !errors.Is(err, config.ErrNoVault) { @@ -99,23 +125,23 @@ func cmdVaultList(args Args) error { seenURL := map[string]bool{} for _, v := range ws.Vaults { seenRoot[filepath.Clean(v.Root)] = true - entries = append(entries, vaultListEntry{Name: v.Name, Kind: "local", Root: v.Root, IsDefault: isDefault("local", v.Root, "")}) + entries = append(entries, vaultListEntry{Name: v.Name, Kind: "local", Source: sourceTerminal, Root: v.Root, IsDefault: isDefault("local", v.Root, "")}) } for _, s := range ws.Servers { - seenURL[strings.ToLower(s.URL)] = true - entries = append(entries, vaultListEntry{Name: s.Name, Kind: "remote", BaseURL: s.URL, IsDefault: isDefault("remote", "", s.URL)}) + seenURL[strings.ToLower(strings.TrimRight(s.URL, "/"))] = true + entries = append(entries, vaultListEntry{Name: s.Name, Kind: "remote", Source: sourceTerminal, BaseURL: s.URL, IsDefault: isDefault("remote", "", s.URL)}) } for _, v := range config.KnownVaults() { if seenRoot[filepath.Clean(v.Root)] { continue } - entries = append(entries, vaultListEntry{Name: v.Name, Kind: "local", Root: v.Root, LastOpenedAt: v.LastOpenedAt, IsDefault: isDefault("local", v.Root, "")}) + entries = append(entries, vaultListEntry{Name: v.Name, Kind: "local", Source: sourceApp, Root: v.Root, LastOpenedAt: v.LastOpenedAt, IsDefault: isDefault("local", v.Root, "")}) } for _, p := range config.RemoteProfiles() { - if seenURL[strings.ToLower(p.BaseURL)] { + if seenURL[strings.ToLower(strings.TrimRight(p.BaseURL, "/"))] { continue } - entries = append(entries, vaultListEntry{Name: p.Name, Kind: "remote", BaseURL: p.BaseURL, LastOpenedAt: p.LastConnectedAt, IsDefault: isDefault("remote", "", p.BaseURL)}) + entries = append(entries, vaultListEntry{Name: p.Name, Kind: "remote", Source: sourceApp, BaseURL: p.BaseURL, LastOpenedAt: p.LastConnectedAt, IsDefault: isDefault("remote", "", p.BaseURL)}) } if args.Bool("json") { emitJSON(entries) @@ -127,11 +153,15 @@ func cmdVaultList(args Args) error { } nameWidth := 4 kindWidth := 0 + hasApp := false for _, e := range entries { nameWidth = max(nameWidth, len([]rune(e.Name))) if e.Kind == "remote" { kindWidth = 6 } + if e.Source == sourceApp { + hasApp = true + } } for _, e := range entries { marker := " " @@ -146,11 +176,19 @@ func cmdVaultList(args Args) error { if kindWidth > 0 { kind = pad(e.Kind, kindWidth) + " " } + source := "" + if hasApp { + source = pad(e.Source, len(sourceTerminal)) + " " + } location := e.Root if location == "" { location = e.BaseURL } - emitLine(fmt.Sprintf("%s %s %s%s %s", marker, pad(e.Name, nameWidth), kind, pad(age, 8), location)) + emitLine(fmt.Sprintf("%s %s %s%s%s %s", marker, pad(e.Name, nameWidth), kind, source, pad(age, 8), location)) + } + if hasApp { + emitLine("") + emitLine("Entries marked app are saved by the ZenNotes desktop app and managed there. `zn use`, `zn disconnect` and `zn vault remove` act on zn's own (terminal) entries.") } return nil } diff --git a/internal/cli/commands_workspace.go b/internal/cli/commands_workspace.go index 6d8b88d..ddcbb9a 100644 --- a/internal/cli/commands_workspace.go +++ b/internal/cli/commands_workspace.go @@ -78,6 +78,9 @@ func cmdConnect(ctx context.Context, args Args) error { if raw == "" { return errors.New("Usage: zn connect [--name ] [--token ] [--no-default]") } + if err := config.CredentialsProblem(); err != nil { + return fmt.Errorf("not connecting: %w", err) + } ws := config.LoadWorkspaces() baseURL := "" name := strings.TrimSpace(args.Str("name")) @@ -153,22 +156,68 @@ func cmdDisconnect(args Args) error { } saved := ws.FindServer(name) if saved == nil { - return fmt.Errorf("No saved server named %q. `zn vault list` shows them.", name) + return notSavedError(ws, name, "server") } url := saved.URL - ws.Remove(saved.Name) - if err := config.SaveWorkspaces(ws); err != nil { + savedName := saved.Name + if err := forgetSavedWorkspace(ws, savedName, url); err != nil { return err } - _ = config.DeleteToken(url) if args.Bool("json") { emitJSON(map[string]any{"ok": true, "name": name, "url": url}) return nil } - emitOK(fmt.Sprintf("Forgot %s (%s)", saved.Name, url)) + emitOK(fmt.Sprintf("Forgot %s (%s)", savedName, url)) return nil } +// notSavedError explains why a selector matched nothing zn saved. `zn vault +// list` also shows the desktop app's entries, so a name copied from there +// gets told who owns it instead of a bare "no such name". +func notSavedError(ws config.Workspaces, selector, noun string) error { + desktop := desktopWorkspaces() + if v := desktop.FindVault(selector); v != nil { + return fmt.Errorf("%q is a vault saved by the ZenNotes desktop app, not by zn; remove it in the app's vault switcher. zn only lists it.%s", v.Name, savedNamesHint(ws)) + } + if s := desktop.FindServer(selector); s != nil { + return fmt.Errorf("%q is a server saved by the ZenNotes desktop app, not by zn; remove it in the app's server settings. zn only lists it.%s", s.Name, savedNamesHint(ws)) + } + hint := savedNamesHint(ws) + if guess := containingName(selector, append(ws.Names(), desktop.Names()...)); guess != "" { + hint = fmt.Sprintf(" Did you mean %q?%s", guess, hint) + } + return fmt.Errorf("No saved %s named %q.%s", noun, selector, hint) +} + +// savedNamesHint names what zn itself saved, which is what `zn use`, +// `zn disconnect` and `zn vault remove` accept. +func savedNamesHint(ws config.Workspaces) string { + names := ws.Names() + if len(names) == 0 { + return " zn has no saved vaults or servers of its own; `zn init`, `zn vault add` and `zn connect` save one." + } + return " zn's saved entries are: " + strings.Join(names, ", ") + ". A server's URL or host works too." +} + +// containingName is the one saved name containing the selector as a word, +// for `zn vault remove workspace` against "workspace (notes.example.com)". +func containingName(selector string, names []string) string { + needle := strings.ToLower(strings.TrimSpace(selector)) + if needle == "" { + return "" + } + match := "" + for _, name := range names { + if strings.Contains(strings.ToLower(name), needle) { + if match != "" { + return "" + } + match = name + } + } + return match +} + // cmdUse makes a saved vault or server the default; `app` follows the // desktop app again; a path or URL is saved first. func cmdUse(ctx context.Context, args Args) error { @@ -186,16 +235,32 @@ func cmdUse(ctx context.Context, args Args) error { return nil } if _, ok := backend.TargetForWorkspace(ws, sel, ""); !ok { + desktop := desktopWorkspaces() switch { case backend.LooksLikeServerURL(sel): return cmdConnect(ctx, args) + case desktop.FindVault(sel) != nil: + // A vault the desktop app knows: save its folder under the same + // name so the terminal can keep using it by that name. + v := desktop.FindVault(sel) + ws.AddVault(v.Name, v.Root) + sel = v.Root + case desktop.FindServer(sel) != nil: + // The app keeps that server's token where zn cannot read it, so + // this is a connect: it asks for the token once. + s := desktop.FindServer(sel) + args.Positionals = []string{s.URL} + if args.Str("name") == "" { + args.push("name", s.Name) + } + return cmdConnect(ctx, args) default: root, err := filepath.Abs(config.ExpandHome(sel)) if err != nil { return err } if info, err := os.Stat(root); err != nil || !info.IsDir() { - return fmt.Errorf("%q is neither a saved name nor a folder. `zn vault list` shows the names.", sel) + return fmt.Errorf("%v `zn use` also takes a folder path or a server URL.", notSavedError(ws, sel, "vault or server")) } ws.AddVault("", root) } @@ -250,30 +315,78 @@ func cmdVaultAdd(args Args) error { return nil } -// cmdVaultRemove forgets a saved vault or server (files stay untouched). +// cmdVaultRemove forgets a saved vault or server. Note files stay; a +// server's token goes with it, as with `zn disconnect`. func cmdVaultRemove(args Args) error { name := strings.TrimSpace(args.Positional(0)) if name == "" { - return errors.New("Usage: zn vault remove ") + return errors.New("Usage: zn vault remove (see `zn vault list`)") } ws := config.LoadWorkspaces() - kind, ok := ws.Remove(name) - if !ok { + var kind, saved, location string + // An exact saved name wins over another entry's folder or host. Resolving + // those aliases first can silently forget the wrong workspace. + for _, v := range ws.Vaults { + if strings.EqualFold(v.Name, name) { + kind, saved, location = "local", v.Name, v.Root + break + } + } + if saved == "" { + for _, s := range ws.Servers { + if strings.EqualFold(s.Name, name) { + kind, saved, location = "remote", s.Name, s.URL + break + } + } + } + if saved == "" { if v := ws.FindVault(name); v != nil { - kind, _ = ws.Remove(v.Name) - ok = true + kind, saved, location = "local", v.Name, v.Root } else if s := ws.FindServer(name); s != nil { - kind, _ = ws.Remove(s.Name) - ok = true + kind, saved, location = "remote", s.Name, s.URL + } else { + return notSavedError(ws, name, "vault or server") } } - if !ok { - return fmt.Errorf("No saved vault or server named %q. `zn vault list` shows them.", name) + baseURL := "" + if kind == "remote" { + baseURL = location + } + if err := forgetSavedWorkspace(ws, saved, baseURL); err != nil { + return err } + if args.Bool("json") { + emitJSON(map[string]any{"ok": true, "name": saved, "kind": kind, "location": location}) + return nil + } + if kind == "remote" { + emitOK(fmt.Sprintf("Forgot server %s (%s) and its token. Nothing on the server changed.", saved, location)) + return nil + } + emitOK(fmt.Sprintf("Forgot vault %s (%s). The notes are still there.", saved, location)) + return nil +} + +// forgetSavedWorkspace keeps the entry retryable if token cleanup fails, and +// restores its token if saving the workspace list fails afterwards. +func forgetSavedWorkspace(ws config.Workspaces, name, baseURL string) error { + token := "" + if baseURL != "" { + token = config.LoadToken(baseURL) + if err := config.DeleteToken(baseURL); err != nil { + return fmt.Errorf("could not remove the saved token: %w", err) + } + } + ws.Remove(name) if err := config.SaveWorkspaces(ws); err != nil { + if token != "" { + if restoreErr := config.SaveToken(baseURL, token); restoreErr != nil { + return errors.Join(err, fmt.Errorf("could not restore the saved token: %w", restoreErr)) + } + } return err } - emitOK(fmt.Sprintf("Forgot %s (%s). Nothing on disk changed.", name, kind)) return nil } diff --git a/internal/cli/completion.go b/internal/cli/completion.go index b0c4e8e..c2c320d 100644 --- a/internal/cli/completion.go +++ b/internal/cli/completion.go @@ -59,11 +59,16 @@ func completionCandidates(words []string) []string { if len(before) > 0 { previous = before[len(before)-1] } + if len(before) >= 2 && before[len(before)-2] == "vault" && (previous == "remove" || previous == "rm") { + previous = "vault remove" + } switch previous { case "--server", "disconnect": for _, s := range config.LoadWorkspaces().Servers { candidates = append(candidates, s.Name) } + case "vault remove": + candidates = config.LoadWorkspaces().Names() case "--vault", "use": candidates = config.LoadWorkspaces().Names() for _, v := range config.KnownVaults() { diff --git a/internal/cli/help.go b/internal/cli/help.go index 28381a7..ff110f7 100644 --- a/internal/cli/help.go +++ b/internal/cli/help.go @@ -13,7 +13,9 @@ import ( const ( terminalColumnsFallback = 80 terminalColumnsCap = 100 - commandColumnWidth = 26 + commandColumnWidth = 28 + // columnGap is the least space between a row's name and its description. + columnGap = 2 ) var ansiRe = regexp.MustCompile(`\x1b\[[0-9;]*m`) @@ -78,7 +80,7 @@ var helpSections = []helpSection{ {"disconnect [name]", "Forget a saved server and its token", "--json"}, {"use ", "Set the terminal's saved default; app follows desktop again. Desktop-installed commands keep following desktop unless --workspace-source terminal is set", "--json"}, {"vault add ", "Remember an existing folder as a vault", "--name --no-default --json"}, - {"vault remove ", "Forget a saved vault or server (files stay)", ""}, + {"vault remove ", "Forget a saved vault or server (its token too; note files stay)", "--json"}, {"vault info", "Vault path (or server) + per-folder counts", "--json"}, {"vault list", "Known vaults and servers; the default is marked with *", "--json"}, {"vault mode [root|inbox]", "Show or switch the notes layout (moves the notes, rewrites favorites)", "--json"}, @@ -262,15 +264,27 @@ func (s helpStyle) header(width int) []string { func (s helpStyle) section(heading string, rows []helpRow, width int) []string { out := []string{s.bold(s.yellow(heading))} descWidth := width - commandColumnWidth - 2 + indent := " " + strings.Repeat(" ", commandColumnWidth) for _, row := range rows { descLines := wrapLines(row.description, descWidth) - out = append(out, " "+padVisible(s.magenta(row.name), commandColumnWidth)+descLines[0]) - for _, cont := range descLines[1:] { - out = append(out, " "+strings.Repeat(" ", commandColumnWidth)+cont) + name := " " + s.magenta(row.name) + if visibleLen(row.name)+columnGap > commandColumnWidth { + // A name too wide for the column gets a line of its own; the + // description still starts at the column, so nothing is glued + // to it and the rows below stay aligned. + out = append(out, name) + for _, line := range descLines { + out = append(out, indent+line) + } + } else { + out = append(out, padVisible(name, len(indent))+descLines[0]) + for _, cont := range descLines[1:] { + out = append(out, indent+cont) + } } if row.flags != "" { for _, line := range wrapLines(row.flags, descWidth) { - out = append(out, " "+strings.Repeat(" ", commandColumnWidth)+s.dim(s.cyan(line))) + out = append(out, indent+s.dim(s.cyan(line))) } } } diff --git a/internal/cli/help_test.go b/internal/cli/help_test.go new file mode 100644 index 0000000..0187988 --- /dev/null +++ b/internal/cli/help_test.go @@ -0,0 +1,87 @@ +package cli + +import ( + "strings" + "testing" +) + +// checkHelpRows asserts every row keeps its description in the description +// column: either on the same line after at least columnGap spaces, or, for a +// name too wide for the column, on the next line at the column. A name must +// never run straight into its description (`""Answer in a thread`). +func checkHelpRows(t *testing.T, lines []string, rows []helpRow) { + t.Helper() + indent := " " + strings.Repeat(" ", commandColumnWidth) + for _, row := range rows { + prefix := " " + row.name + idx := -1 + for i, line := range lines { + if line == prefix || strings.HasPrefix(line, prefix+" ") { + idx = i + break + } + } + if idx < 0 { + t.Errorf("%q: not rendered as its own row (glued to its description?)", row.name) + continue + } + firstWord := strings.Fields(row.description)[0] + rest := strings.TrimPrefix(lines[idx], prefix) + if strings.TrimSpace(rest) == "" { + if idx+1 >= len(lines) || !strings.HasPrefix(lines[idx+1], indent+firstWord) { + t.Errorf("%q: description should start on the next line at column %d, got %q", row.name, len(indent), lines[idx+1]) + } + continue + } + column := len(lines[idx]) - len(strings.TrimLeft(rest, " ")) + if column != len(indent) || !strings.HasPrefix(strings.TrimLeft(rest, " "), firstWord) { + t.Errorf("%q: description starts at column %d, want %d: %q", row.name, column, len(indent), lines[idx]) + } + if len(rest)-len(strings.TrimLeft(rest, " ")) < columnGap { + t.Errorf("%q: fewer than %d spaces before the description: %q", row.name, columnGap, lines[idx]) + } + } +} + +func TestHelpRowsNeverGlueNameToDescription(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("FORCE_COLOR", "") + rows := append([]helpRow{}, globalFlags...) + rows = append(rows, environmentRows...) + for _, sec := range helpSections { + rows = append(rows, sec.rows...) + } + checkHelpRows(t, strings.Split(RenderHelp(nil), "\n"), rows) + + // Scoped help renders the global flags through the same layout. + for _, name := range []string{"comment reply", "list", "server"} { + lines := strings.Split(RenderScopedHelp(name, nil), "\n") + start := -1 + for i, line := range lines { + if line == "GLOBAL FLAGS" { + start = i + } + } + if start < 0 { + t.Fatalf("scoped help for %q has no GLOBAL FLAGS section", name) + } + checkHelpRows(t, lines[start:], globalFlags) + } +} + +func TestHelpWrapsLongDescriptionsAtTheColumn(t *testing.T) { + t.Setenv("NO_COLOR", "1") + t.Setenv("FORCE_COLOR", "") + out := RenderHelp(nil) + indent := " " + strings.Repeat(" ", commandColumnWidth) + for _, line := range strings.Split(out, "\n") { + if strings.HasPrefix(line, indent+" ") { + t.Errorf("continuation line indented past the column: %q", line) + } + } + for _, glued := range []string{`""Answer`, `""Start`, `Resolve`, `Follow`} { + if strings.Contains(out, glued) { + t.Errorf("help still glues a name to its description: %q", glued) + } + } +} diff --git a/internal/cli/pretty.go b/internal/cli/pretty.go index 6e17dde..19583e2 100644 --- a/internal/cli/pretty.go +++ b/internal/cli/pretty.go @@ -7,8 +7,9 @@ import ( "github.com/charmbracelet/glamour" "github.com/charmbracelet/glamour/styles" - "github.com/charmbracelet/lipgloss" "golang.org/x/term" + + "github.com/ZenNotes/tui/internal/termbg" ) // renderPretty formats a note through Glamour for the terminal: the @@ -28,7 +29,7 @@ func renderPretty(body, style string) (string, error) { case !isTTY && (style == "" || style == "auto"): opts = append(opts, glamour.WithStandardStyle(styles.NoTTYStyle)) case style == "" || style == "auto": - if lipgloss.HasDarkBackground() { + if termbg.Detect() { opts = append(opts, glamour.WithStandardStyle(styles.DarkStyle)) } else { opts = append(opts, glamour.WithStandardStyle(styles.LightStyle)) diff --git a/internal/cli/registry.go b/internal/cli/registry.go index 41b82b6..5aac676 100644 --- a/internal/cli/registry.go +++ b/internal/cli/registry.go @@ -177,6 +177,9 @@ func validatePositionals(spec commandSpec, args Args) error { if spec.Name == "task toggle" && args.Str("id") != "" { minimum-- } + if spec.Name == "tag find" && args.Str("tag") != "" { + minimum-- + } if spec.Name == "base create" && args.Str("title") != "" { minimum-- } @@ -216,7 +219,7 @@ func parseFlag(argv []string, i int, allowed map[string]bool, args *Args) (int, return i, fmt.Errorf("unknown flag --%s.%s", name, suggestion(name, choices)) } if takesValue && !assigned { - if i+1 >= len(argv) || strings.HasPrefix(argv[i+1], "--") || argv[i+1] == "-h" { + if i+1 >= len(argv) || startsAFlag(argv[i+1]) { return i, fmt.Errorf("--%s needs a value", name) } i++ @@ -275,25 +278,8 @@ func suggestion(input string, choices []string) string { sort.Strings(choices) best, distance := "", 3 for _, choice := range choices { - row := make([]int, len(choice)+1) - for j := range row { - row[j] = j - } - for i := range input { - prev := row[0] - row[0] = i + 1 - for j := range choice { - cost := 0 - if input[i] != choice[j] { - cost = 1 - } - old := row[j+1] - row[j+1] = min(row[j+1]+1, row[j]+1, prev+cost) - prev = old - } - } - if row[len(choice)] < distance { - best, distance = choice, row[len(choice)] + if d := editDistance(input, choice); d < distance { + best, distance = choice, d } } if best == "" { @@ -301,3 +287,31 @@ func suggestion(input string, choices []string) string { } return " Did you mean " + strconv.Quote(best) + "?" } + +// editDistance counts insertions, deletions, substitutions and swaps of +// adjacent characters, so `lsit` is one edit from `list` rather than two +// (which tied it with `init` and lost on alphabetical order). +func editDistance(a, b string) int { + rows, cols := len(a)+1, len(b)+1 + d := make([][]int, rows) + for i := range d { + d[i] = make([]int, cols) + d[i][0] = i + } + for j := 0; j < cols; j++ { + d[0][j] = j + } + for i := 1; i < rows; i++ { + for j := 1; j < cols; j++ { + cost := 0 + if a[i-1] != b[j-1] { + cost = 1 + } + d[i][j] = min(d[i-1][j]+1, d[i][j-1]+1, d[i-1][j-1]+cost) + if i > 1 && j > 1 && a[i-1] == b[j-2] && a[i-2] == b[j-1] { + d[i][j] = min(d[i][j], d[i-2][j-2]+1) + } + } + } + return d[rows-1][cols-1] +} diff --git a/internal/cli/vault_remove_test.go b/internal/cli/vault_remove_test.go new file mode 100644 index 0000000..4ec80a5 --- /dev/null +++ b/internal/cli/vault_remove_test.go @@ -0,0 +1,301 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/ZenNotes/tui/internal/config" +) + +// The desktop app's vaults and servers appear in `zn vault list` next to +// zn's own, but only zn's own can be forgotten here. A name copied from the +// list must be told who owns it and what zn would accept instead. +func TestVaultRemoveExplainsDesktopEntriesAndListsZnsOwn(t *testing.T) { + dir := isolatedCLI(t) + root := filepath.Join(dir, "notes") + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + app, _ := json.Marshal(map[string]any{ + "workspaceMode": "remote", + "localVaults": []map[string]any{{"name": "vault", "root": root, "lastOpenedAt": 1700000000000}}, + "remoteWorkspaceProfiles": []map[string]any{{ + "name": "workspace (zennotes.mydomain.com)", "baseUrl": "https://zennotes.mydomain.com", "lastConnectedAt": 1700000000000, + }}, + }) + if err := os.WriteFile(filepath.Join(config.UserDataDir(), config.AppConfigFile), app, 0o600); err != nil { + t.Fatal(err) + } + ws := config.LoadWorkspaces() + ws.AddVault("alpha", filepath.Join(dir, "alpha")) + ws.AddServer("", "https://zennotes.mydomain.com") + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + if err := config.SaveToken("https://zennotes.mydomain.com", "secret"); err != nil { + t.Fatal(err) + } + + out := captureOutput(t, func() { _ = Main([]string{"vault", "list"}) }) + for _, want := range []string{"alpha", "terminal", "vault", "app", "Entries marked app"} { + if !strings.Contains(out, want) { + t.Errorf("vault list should show %q:\n%s", want, out) + } + } + out = captureOutput(t, func() { _ = Main([]string{"vault", "list", "--json"}) }) + var entries []vaultListEntry + if err := json.Unmarshal([]byte(out), &entries); err != nil { + t.Fatal(err) + } + sources := map[string]string{} + for _, e := range entries { + sources[e.Name] = e.Source + } + if sources["alpha"] != "terminal" || sources["zennotes.mydomain.com"] != "terminal" || sources["vault"] != "app" { + t.Fatalf("sources: %v", sources) + } + if _, listed := sources["workspace (zennotes.mydomain.com)"]; listed { + t.Fatal("the desktop profile for a URL zn saved must not be listed twice") + } + + // Kevin's attempts, in order. + code := 0 + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "workspace"}) }) + if code != 1 || !strings.Contains(out, `Did you mean "workspace (zennotes.mydomain.com)"`) || !strings.Contains(out, "zn's saved entries are: alpha, zennotes.mydomain.com") { + t.Fatalf("partial name: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "vault"}) }) + if code != 1 || !strings.Contains(out, "saved by the ZenNotes desktop app") { + t.Fatalf("desktop vault: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "zennotes.mydomain.com", "--json"}) }) + if code != 0 || !strings.Contains(out, `"kind": "remote"`) { + t.Fatalf("remove by host: exit %d %s", code, out) + } + if config.LoadWorkspaces().FindServer("zennotes.mydomain.com") != nil { + t.Fatal("server still saved") + } + if config.LoadToken("https://zennotes.mydomain.com") != "" { + t.Fatal("vault remove must drop the server's token like disconnect does") + } + // Now only the desktop profile knows that URL. + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "zennotes.mydomain.com"}) }) + if code != 1 || !strings.Contains(out, "server saved by the ZenNotes desktop app") { + t.Fatalf("desktop server by host: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "alpha"}) }) + if code != 0 || !strings.Contains(out, "Forgot vault alpha") { + t.Fatalf("remove vault: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"vault", "remove", "alpha"}) }) + if code != 1 || !strings.Contains(out, "zn has no saved vaults or servers of its own") { + t.Fatalf("nothing left: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"disconnect", "workspace (zennotes.mydomain.com)"}) }) + if code != 1 || !strings.Contains(out, "saved by the ZenNotes desktop app") { + t.Fatalf("disconnect desktop profile: exit %d %s", code, out) + } +} + +// `zn use ` adopts the folder the app knows under that +// name instead of failing with "neither a saved name nor a folder". +func TestUseAdoptsADesktopVaultByName(t *testing.T) { + dir := isolatedCLI(t) + root := filepath.Join(dir, "desktop-notes") + if err := os.MkdirAll(root, 0o755); err != nil { + t.Fatal(err) + } + app, _ := json.Marshal(map[string]any{"localVaults": []map[string]any{{"name": "Desk", "root": root}}}) + if err := os.WriteFile(filepath.Join(config.UserDataDir(), config.AppConfigFile), app, 0o600); err != nil { + t.Fatal(err) + } + code := 0 + out := captureOutput(t, func() { code = Main([]string{"use", "desk", "--json"}) }) + if code != 0 || !strings.Contains(out, `"default": "Desk"`) { + t.Fatalf("use: exit %d %s", code, out) + } + if v := config.LoadWorkspaces().FindVault("Desk"); v == nil || v.Root != root { + t.Fatalf("not adopted: %+v", config.LoadWorkspaces()) + } + out = captureOutput(t, func() { code = Main([]string{"use", "nothing-like-it"}) }) + if code != 1 || !strings.Contains(out, "zn's saved entries are: Desk") || !strings.Contains(out, "folder path or a server URL") { + t.Fatalf("unknown: exit %d %s", code, out) + } +} + +func TestVaultRemoveCompletesZnsOwnNames(t *testing.T) { + dir := isolatedCLI(t) + ws := config.LoadWorkspaces() + ws.AddVault("alpha", filepath.Join(dir, "a")) + ws.AddServer("home", "http://127.0.0.1:7878") + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + for _, verb := range []string{"remove", "rm"} { + out := captureOutput(t, func() { _ = Main([]string{"__complete", "--", "vault", verb, ""}) }) + if strings.TrimSpace(out) != "alpha\nhome" { + t.Fatalf("vault %s completion: %q", verb, out) + } + } +} + +func TestVaultRemovePrefersSavedNamesOverPathsAndHosts(t *testing.T) { + for _, kind := range []string{"local", "remote", "remote host"} { + t.Run(kind, func(t *testing.T) { + dir := isolatedCLI(t) + t.Chdir(dir) + ws := config.Workspaces{} + if kind == "remote host" { + ws.AddServer("keep", "https://home") + } else { + ws.AddVault("keep", filepath.Join(dir, "home")) + } + if kind == "local" { + ws.AddVault("home", filepath.Join(dir, "other")) + } else { + ws.AddServer("home", "https://notes.example.com") + if err := config.SaveToken("https://notes.example.com", "token"); err != nil { + t.Fatal(err) + } + } + ws.Default = "keep" + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + code := 0 + out := captureOutput(t, func() { code = Main([]string{"vault", "remove", "home", "--json"}) }) + if code != 0 { + t.Fatalf("remove: exit %d %s", code, out) + } + remaining := config.LoadWorkspaces() + if names := remaining.Names(); len(names) != 1 || names[0] != "keep" || remaining.Default != "keep" { + t.Fatalf("removed the wrong entry: %+v; output: %s", remaining, out) + } + if kind != "local" && config.LoadToken("https://notes.example.com") != "" { + t.Fatal("the selected server's token was not deleted") + } + }) + } +} + +func TestForgetServerReportsStorageFailuresAndCanBeRetried(t *testing.T) { + for _, command := range [][]string{{"vault", "remove", "home"}, {"disconnect", "home"}} { + for _, problem := range []string{"corrupt", "write failure", "workspace write failure"} { + t.Run(strings.Join(command, " ")+"/"+problem, func(t *testing.T) { + isolatedCLI(t) + ws := config.Workspaces{Default: "home"} + ws.AddServer("home", "https://notes.example.com") + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + if err := config.SaveToken("https://notes.example.com", "token"); err != nil { + t.Fatal(err) + } + original, err := os.ReadFile(config.CredentialsPath()) + if err != nil { + t.Fatal(err) + } + broken := []byte("[tokens]\n\"https://notes.example.com\" = \"token\n") + failedPath := config.CredentialsPath() + if problem == "workspace write failure" { + failedPath = config.WorkspacesPath() + } + if problem == "corrupt" { + err = os.WriteFile(config.CredentialsPath(), broken, 0o600) + } else { + err = os.Mkdir(failedPath+".tmp", 0o700) + } + if err != nil { + t.Fatal(err) + } + code := 0 + out := captureOutput(t, func() { code = Main(append(command, "--json")) }) + if code == 0 || !strings.Contains(out, filepath.Base(failedPath)) { + t.Errorf("must report failed removal: exit %d %s", code, out) + } + if saved := config.LoadWorkspaces(); saved.FindServer("home") == nil || saved.Default != "home" { + t.Fatalf("must keep the entry so removal can be retried: %+v", saved) + } + want := original + if problem == "corrupt" { + want = broken + } + if raw, err := os.ReadFile(config.CredentialsPath()); err != nil || string(raw) != string(want) { + t.Fatalf("credentials changed on failure: %q %v", raw, err) + } + if problem == "corrupt" { + err = os.WriteFile(config.CredentialsPath(), original, 0o600) + } else { + err = os.Remove(failedPath + ".tmp") + } + if err != nil { + t.Fatal(err) + } + out = captureOutput(t, func() { code = Main(command) }) + if code != 0 || config.LoadWorkspaces().FindServer("home") != nil || config.LoadToken("https://notes.example.com") != "" { + t.Fatalf("retry did not remove the server and token: exit %d %s", code, out) + } + }) + } + } +} + +// With an unparseable workspaces.toml, every command that would save must +// refuse rather than write an empty list over the user's saved entries. +func TestCommandsRefuseToOverwriteABrokenWorkspacesFile(t *testing.T) { + dir := isolatedCLI(t) + ws := config.LoadWorkspaces() + ws.AddVault("alpha", filepath.Join(dir, "alpha")) + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + broken := []byte("default = \"alpha\"\n[[vault]]\nname = \"alpha\"\nroot = \"/a\"\n[vault]\nname = \"oops\"\n") + if err := os.WriteFile(config.WorkspacesPath(), broken, 0o644); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Join(dir, "beta"), 0o755); err != nil { + t.Fatal(err) + } + code := 0 + out := captureOutput(t, func() { code = Main([]string{"vault", "add", filepath.Join(dir, "beta"), "--name", "beta"}) }) + if code == 0 || !strings.Contains(out, "not saving") || !strings.Contains(out, "workspaces.toml") { + t.Fatalf("vault add: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"init", filepath.Join(dir, "gamma")}) }) + if code == 0 || !strings.Contains(out, "not saving") { + t.Fatalf("init: exit %d %s", code, out) + } + if raw, _ := os.ReadFile(config.WorkspacesPath()); string(raw) != string(broken) { + t.Fatal("workspaces.toml was overwritten") + } + out = captureOutput(t, func() { code = Main([]string{"doctor", "--json"}) }) + var report struct { + Checks []diagnosticCheck `json:"checks"` + } + if err := json.Unmarshal([]byte(strings.SplitN(out, "\n{\"error\"", 2)[0]), &report); err != nil { + t.Fatalf("doctor json: %v %s", err, out) + } + found := false + for _, c := range report.Checks { + if c.Name == "saved vaults" { + found = true + if c.OK || !strings.Contains(c.Message, "workspaces.toml") { + t.Fatalf("saved vaults check: %+v", c) + } + } + } + if !found || code == 0 { + t.Fatalf("doctor must fail on a broken workspaces file: exit %d %s", code, out) + } + out = captureOutput(t, func() { code = Main([]string{"status"}) }) + if !strings.Contains(out, "Warning: saved vaults and servers are unavailable") { + t.Fatalf("status warning: %s", out) + } + out = captureOutput(t, func() { _ = Main([]string{"vault", "list"}) }) + if !strings.Contains(out, "zn's own vaults and servers are not shown") { + t.Fatalf("vault list warning: %s", out) + } +} diff --git a/internal/cli/workspace_test.go b/internal/cli/workspace_test.go index dd36d7f..dea2771 100644 --- a/internal/cli/workspace_test.go +++ b/internal/cli/workspace_test.go @@ -103,6 +103,35 @@ func TestConnectInitUseAndList(t *testing.T) { } } +func TestConnectWithBrokenCredentialsKeepsTheSavedDefault(t *testing.T) { + dir := isolatedCLI(t) + ws := config.Workspaces{Default: "notes"} + ws.AddVault("notes", dir) + if err := config.SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + original, err := os.ReadFile(config.WorkspacesPath()) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(config.CredentialsPath(), []byte("[tokens]\n\"https://other.example.com\" = \"unterminated\n"), 0o600); err != nil { + t.Fatal(err) + } + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"root":"/srv/notes","name":"notes"}`)) + })) + defer server.Close() + code := 0 + out := captureOutput(t, func() { code = Main([]string{"connect", server.URL, "--token", "right", "--json"}) }) + if code == 0 || !strings.Contains(out, "credentials.toml") { + t.Fatalf("must report the broken store: exit %d %s", code, out) + } + if raw, err := os.ReadFile(config.WorkspacesPath()); err != nil || string(raw) != string(original) { + t.Fatalf("failed connection changed saved workspaces: %s %v", raw, err) + } +} + // A desktop-managed zn follows the app, so `zn connect` must not claim zn // now uses the server by default; what it does is give zn (and zn mcp) the // token for the server whenever the app has it open. diff --git a/internal/config/workspaces.go b/internal/config/workspaces.go index 27bc45a..7709cc8 100644 --- a/internal/config/workspaces.go +++ b/internal/config/workspaces.go @@ -39,6 +39,11 @@ type Workspaces struct { Default string `toml:"default"` Vaults []LocalWorkspace `toml:"vault"` Servers []ServerWorkspace `toml:"server"` + // loadErr remembers that the file on disk exists but could not be read + // or parsed. The list is then empty, and SaveWorkspaces refuses to + // replace the file with it: a stray edit must not cost the user every + // saved vault and server on the next `zn connect`. + loadErr error } // WorkspacesPath is where zn's vault and server list lives. @@ -47,19 +52,37 @@ func WorkspacesPath() string { return filepath.Join(PortableConfigDir(), "worksp // CredentialsPath is where server tokens live (mode 0600). func CredentialsPath() string { return filepath.Join(PortableConfigDir(), "credentials.toml") } -// LoadWorkspaces reads the list; a missing file is an empty list. +// LoadWorkspaces reads the list; a missing or unreadable file is an empty +// list. LoadWorkspacesFile also reports the latter. func LoadWorkspaces() Workspaces { + ws, _ := LoadWorkspacesFile() + return ws +} + +// LoadWorkspacesFile reads the list and says when the file exists but +// could not be read or parsed; a missing file is simply an empty list. +func LoadWorkspacesFile() (Workspaces, error) { var ws Workspaces raw, err := os.ReadFile(WorkspacesPath()) + if errors.Is(err, os.ErrNotExist) { + return ws, nil + } + if err == nil { + _, err = toml.Decode(string(raw), &ws) + } if err != nil { - return ws + ws = Workspaces{loadErr: fmt.Errorf("%s: %w", WorkspacesPath(), err)} + return ws, ws.loadErr } - _, _ = toml.Decode(string(raw), &ws) - return ws + return ws, nil } -// SaveWorkspaces writes the list atomically. +// SaveWorkspaces writes the list atomically. A list that stands in for a +// file zn could not read is never written back over it. func SaveWorkspaces(ws Workspaces) error { + if ws.loadErr != nil { + return fmt.Errorf("not saving: %v. Overwriting the file would lose the vaults and servers it lists; fix or move it, then retry.", ws.loadErr) + } var buf bytes.Buffer buf.WriteString("# Vaults and servers zn knows. `zn connect`, `zn vault add` and `zn use` edit it.\n") if err := toml.NewEncoder(&buf).Encode(ws); err != nil { @@ -70,22 +93,65 @@ func SaveWorkspaces(ws Workspaces) error { type credentials struct { Tokens map[string]string `toml:"tokens"` + // loadErr: see Workspaces.loadErr; a token store zn could not read is + // never overwritten with a fresh one holding a single token. + loadErr error } func loadCredentials() credentials { + c, _ := loadCredentialsFile() + return c +} + +func loadCredentialsFile() (credentials, error) { c := credentials{Tokens: map[string]string{}} raw, err := os.ReadFile(CredentialsPath()) + if errors.Is(err, os.ErrNotExist) { + return c, nil + } + if err == nil { + var md toml.MetaData + md, err = toml.Decode(string(raw), &c) + if err != nil { + // Parser messages and key names can contain the token itself. Keep + // only the location; these errors also appear in doctor output. + var parseErr toml.ParseError + if errors.As(err, &parseErr) { + err = fmt.Errorf("invalid credentials TOML at line %d; fix or move the file, then retry", parseErr.Position.Line) + } else { + err = errors.New("invalid credentials TOML; fix or move the file, then retry") + } + } else { + // The decoder silently ignores non-table values assigned to a map. + // Treat that as a broken store instead of overwriting it as empty. + for _, key := range md.Keys() { + if len(key) == 1 && strings.EqualFold(key[0], "tokens") && md.Type(key...) != "Hash" { + err = errors.New("credentials must contain a [tokens] table; fix or move the file, then retry") + break + } + } + } + } if err != nil { - return c + c = credentials{Tokens: map[string]string{}, loadErr: fmt.Errorf("%s: %w", CredentialsPath(), err)} + return c, c.loadErr } - _, _ = toml.Decode(string(raw), &c) if c.Tokens == nil { c.Tokens = map[string]string{} } - return c + return c, nil +} + +// CredentialsProblem says when the token store exists but cannot be read. +func CredentialsProblem() error { + _, err := loadCredentialsFile() + return err } func saveCredentials(c credentials) error { + if c.loadErr != nil { + return fmt.Errorf("not saving: %v. Overwriting the file would lose the other saved tokens; fix or move it, then retry.", c.loadErr) + } var buf bytes.Buffer buf.WriteString("# Server tokens for zn, keyed by URL. Keep this file private.\n") if err := toml.NewEncoder(&buf).Encode(c); err != nil { @@ -108,7 +174,10 @@ func SaveToken(baseURL, token string) error { // DeleteToken forgets a server token. func DeleteToken(baseURL string) error { - c := loadCredentials() + c, err := loadCredentialsFile() + if err != nil { + return err + } if _, ok := c.Tokens[tokenKey(baseURL)]; !ok { return nil } diff --git a/internal/config/workspaces_test.go b/internal/config/workspaces_test.go index a69a033..928aadd 100644 --- a/internal/config/workspaces_test.go +++ b/internal/config/workspaces_test.go @@ -4,6 +4,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" ) @@ -49,3 +50,107 @@ func TestWorkspacesRoundTripAndTokens(t *testing.T) { t.Fatal("token deleted") } } + +// A workspaces.toml zn cannot parse must not be mistaken for an empty list +// and overwritten by the next command that saves: that would delete every +// vault and server the user connected. +func TestCorruptWorkspacesFileIsNeverOverwritten(t *testing.T) { + dir := t.TempDir() + t.Setenv("ZENNOTES_CONFIG_DIR", dir) + ws := Workspaces{} + ws.AddVault("alpha", filepath.Join(dir, "a")) + ws.AddServer("home", "https://notes.example.com") + if err := SaveWorkspaces(ws); err != nil { + t.Fatal(err) + } + original, _ := os.ReadFile(WorkspacesPath()) + broken := append(original, []byte("\n[server]\nname = \"x\"\n")...) // a table where an array of tables is expected + if err := os.WriteFile(WorkspacesPath(), broken, 0o644); err != nil { + t.Fatal(err) + } + + loaded, err := LoadWorkspacesFile() + if err == nil || len(loaded.Vaults) != 0 || len(loaded.Servers) != 0 { + t.Fatalf("a broken file must report its problem and list nothing: %+v %v", loaded, err) + } + loaded.AddVault("beta", filepath.Join(dir, "b")) + if err := SaveWorkspaces(loaded); err == nil || !strings.Contains(err.Error(), "not saving") { + t.Fatalf("save must refuse: %v", err) + } + after, _ := os.ReadFile(WorkspacesPath()) + if string(after) != string(broken) { + t.Fatal("the broken file was replaced") + } + + // Repaired, everything is back and saving works again. + if err := os.WriteFile(WorkspacesPath(), original, 0o644); err != nil { + t.Fatal(err) + } + repaired, err := LoadWorkspacesFile() + if err != nil || len(repaired.Vaults) != 1 || len(repaired.Servers) != 1 { + t.Fatalf("repaired: %+v %v", repaired, err) + } + if err := SaveWorkspaces(repaired); err != nil { + t.Fatal(err) + } + if _, err := LoadWorkspacesFile(); err != nil { + t.Fatal(err) + } +} + +func TestCorruptCredentialsFileIsNeverOverwritten(t *testing.T) { + dir := t.TempDir() + t.Setenv("ZENNOTES_CONFIG_DIR", dir) + if err := SaveToken("https://a.example.com", "token-a"); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(CredentialsPath(), []byte("[tokens]\n\"https://a.example.com\" = \"token-a\n"), 0o600); err != nil { + t.Fatal(err) + } + if CredentialsProblem() == nil { + t.Fatal("problem not reported") + } + if LoadToken("https://a.example.com") != "" { + t.Fatal("a broken store must not yield tokens") + } + if err := SaveToken("https://b.example.com", "token-b"); err == nil || !strings.Contains(err.Error(), "not saving") { + t.Fatalf("save must refuse: %v", err) + } + raw, _ := os.ReadFile(CredentialsPath()) + if string(raw) != "[tokens]\n\"https://a.example.com\" = \"token-a\n" { + t.Fatal("the broken store was replaced") + } + if err := DeleteToken("https://a.example.com"); err == nil { + t.Fatal("deleting from a broken store must report that the token could not be removed") + } +} + +func TestCredentialErrorsDoNotExposeFileContents(t *testing.T) { + const secret = "testsecrettoken" + for name, broken := range map[string]string{ + "unquoted value": "[tokens]\n\"https://notes.example.com\" = " + secret + "\n", + "token as key": "[tokens]\n" + secret + " = \"unterminated\n", + "wrong type": "tokens = \"" + secret + "\"\n", + } { + t.Run(name, func(t *testing.T) { + t.Setenv("ZENNOTES_CONFIG_DIR", t.TempDir()) + if err := os.WriteFile(CredentialsPath(), []byte(broken), 0o600); err != nil { + t.Fatal(err) + } + for operation, err := range map[string]error{ + "diagnose": CredentialsProblem(), + "save": SaveToken("https://other.example.com", "other"), + "delete": DeleteToken("https://notes.example.com"), + } { + if err == nil || !strings.Contains(err.Error(), "credentials.toml") { + t.Errorf("%s: expected an actionable credentials error, got %v", operation, err) + } else if strings.Contains(err.Error(), secret) { + t.Errorf("%s exposed a credential in the diagnostic: %v", operation, err) + } + } + if raw, err := os.ReadFile(CredentialsPath()); err != nil || string(raw) != broken { + t.Fatalf("invalid credentials were overwritten: %q %v", raw, err) + } + }) + } +} diff --git a/internal/database/doc.go b/internal/database/doc.go index 4a4046c..bceabdf 100644 --- a/internal/database/doc.go +++ b/internal/database/doc.go @@ -245,7 +245,8 @@ func (d *Doc) RecordTitle(row Row) string { } // ComposePageBody composes a record page: the row's properties as flat YAML -// frontmatter (id and title fields omitted) followed by body. +// frontmatter (id and title fields omitted) followed by body. A row with no +// properties to mirror gets no frontmatter block at all. func (d *Doc) ComposePageBody(row Row, body string) string { titleID := d.TitleFieldID() lines := []string{"---"} @@ -260,6 +261,9 @@ func (d *Doc) ComposePageBody(row Row, body string) string { lines = append(lines, f.Name+":") } } + if len(lines) == 1 { + return strings.TrimLeft(body, "\n") + } lines = append(lines, "---") return strings.Join(lines, "\n") + "\n" + strings.TrimLeft(body, "\n") } diff --git a/internal/database/schema_test.go b/internal/database/schema_test.go index d91e548..5e8f22a 100644 --- a/internal/database/schema_test.go +++ b/internal/database/schema_test.go @@ -3,6 +3,8 @@ package database import ( "strings" "testing" + + "github.com/ZenNotes/tui/internal/vault" ) func testDoc(t *testing.T) *Doc { @@ -97,3 +99,40 @@ func TestNoteLinksAndDuplicate(t *testing.T) { t.Fatalf("duplicate: %+v", d.Rows) } } + +// A record page for a row with nothing but a title has no properties to +// mirror, so it gets no frontmatter block; re-mirroring is idempotent either +// way instead of stacking `---` fences on every `zn base set`. +func TestComposePageBodyIsIdempotentWithoutProperties(t *testing.T) { + d := testDoc(t) + if err := d.DeleteField("f2"); err != nil { + t.Fatal(err) + } + row := d.Rows[0] + page := d.ComposePageBody(row, "# Dune\n\nbody\n") + if page != "# Dune\n\nbody\n" { + t.Fatalf("page without properties: %q", page) + } + _, body, _ := vault.Frontmatter(page) + if again := d.ComposePageBody(row, body); again != page { + t.Fatalf("re-mirror changed the page: %q", again) + } + // Pages written by older builds carry an empty block; it is recognised + // as frontmatter and replaced, not stacked. + _, body, ok := vault.Frontmatter("---\n---\n---\n---\n# Dune\n\nbody\n") + if !ok { + t.Fatal("empty frontmatter block not recognised") + } + if again := d.ComposePageBody(row, body); again != "---\n---\n# Dune\n\nbody\n" { + t.Fatalf("one stray block should remain to be stripped on the next pass, got %q", again) + } + full := testDoc(t) + withProps := full.ComposePageBody(full.Rows[0], "# Dune\n\nbody\n") + if withProps != "---\nStatus: todo\n---\n# Dune\n\nbody\n" { + t.Fatalf("page with properties: %q", withProps) + } + _, body, _ = vault.Frontmatter(withProps) + if again := full.ComposePageBody(full.Rows[0], body); again != withProps { + t.Fatalf("re-mirror with properties changed the page: %q", again) + } +} diff --git a/internal/mcp/run_test.go b/internal/mcp/run_test.go index 3641000..412a63f 100644 --- a/internal/mcp/run_test.go +++ b/internal/mcp/run_test.go @@ -3,6 +3,9 @@ package mcp import ( "context" "encoding/json" + "errors" + "fmt" + "io" "net/http" "net/http/httptest" "os" @@ -16,6 +19,57 @@ import ( "github.com/ZenNotes/tui/internal/backend" ) +type failingTransport struct{ err error } + +func (t failingTransport) Connect(context.Context) (sdk.Connection, error) { + return nil, t.err +} + +func TestRunOnlySuppressesNormalShutdownErrors(t *testing.T) { + for _, tc := range []struct { + name string + err error + ok bool + }{ + {"EOF", io.EOF, true}, + {"wrapped EOF", fmt.Errorf("read: %w", io.EOF), true}, + {"SDK EOF", errors.New("server is closing: EOF"), true}, + {"canceled", context.Canceled, true}, + {"deadline", context.DeadlineExceeded, false}, + {"truncated input", errors.New("server is closing: unexpected EOF"), false}, + {"transport failure", errors.New("server is closing: input/output error"), false}, + } { + t.Run(tc.name, func(t *testing.T) { + err := Run(context.Background(), Options{Transport: failingTransport{tc.err}}) + if tc.ok && err != nil { + t.Fatalf("normal shutdown must be quiet: %v", err) + } + if !tc.ok && !errors.Is(err, tc.err) { + t.Fatalf("lost transport failure: got %v, want %v", err, tc.err) + } + }) + } +} + +func TestRunReturnsOnClientDisconnect(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + serverT, clientT := sdk.NewInMemoryTransports() + done := make(chan error, 1) + go func() { done <- Run(ctx, Options{Transport: serverT}) }() + client := sdk.NewClient(&sdk.Implementation{Name: "test", Version: "0"}, nil) + cs, err := client.Connect(ctx, clientT, nil) + if err != nil { + t.Fatal(err) + } + if err := cs.Close(); err != nil { + t.Fatal(err) + } + if err := <-done; err != nil { + t.Fatalf("client disconnect must be quiet: %v", err) + } +} + // runServer serves Run over an in-memory transport and returns a connected // client; resolve is consulted before every tool call, like the app's config. func runServer(t *testing.T, resolve func() (backend.Target, error)) *sdk.ClientSession { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 2d896e1..258a602 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -11,6 +11,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "sort" "strings" @@ -1093,7 +1094,19 @@ func Run(ctx context.Context, opts Options) error { if transport == nil { transport = &sdk.StdioTransport{} } - return server.Run(ctx, transport) + err := server.Run(ctx, transport) + if err == nil || errors.Is(err, context.Canceled) || isClientDisconnect(err) { + return nil + } + return err +} + +// isClientDisconnect recognises the SDK's report that the client closed +// the pipe, which is how MCP clients end a session: not an error. +func isClientDisconnect(err error) bool { + // The SDK sometimes formats EOF with %v rather than wrapping it. Match + // that exact fallback; the same prefix can accompany real I/O failures. + return errors.Is(err, io.EOF) || err.Error() == "server is closing: EOF" } func errorResult(text string) *sdk.CallToolResult { diff --git a/internal/server/lifecycle.go b/internal/server/lifecycle.go index 31ae915..c216422 100644 --- a/internal/server/lifecycle.go +++ b/internal/server/lifecycle.go @@ -340,7 +340,12 @@ func (m *Manager) Run(ctx context.Context, name string, output io.Writer) error } cmd.Stdout, cmd.Stderr = output, output if err := cmd.Run(); err != nil { - return errors.Join(err, ctx.Err()) + if ctx.Err() != nil { + // Ctrl-C: the operator stopped the server, which has already + // logged its shutdown; that is not an error to report twice. + return nil + } + return err } return nil } diff --git a/internal/termbg/termbg.go b/internal/termbg/termbg.go new file mode 100644 index 0000000..1045e98 --- /dev/null +++ b/internal/termbg/termbg.go @@ -0,0 +1,66 @@ +// Package termbg decides the terminal's background color for Lip Gloss +// before Bubble Tea can ask the terminal itself. +// +// Bubble Tea v1 queries the terminal in a package init (tea_init.go: "make +// sure Lip Gloss and Termenv query the terminal before any Bubble Tea +// Program runs") whenever stdout is a terminal. Termenv waits up to +// OSCTimeout, five seconds, for the answer. Terminals that never answer +// OSC 11 (the Linux console, some IDE and serial terminals, a few +// multiplexer setups) therefore stalled every zn command, `zn --version` +// included, for five seconds, and the belated answer could land in the +// next thing reading stdin, such as the `zn setup` prompt. +// +// Go initializes packages in import-path order among those whose imports +// are ready, and github.com/ZenNotes sorts before github.com/charmbracelet, +// so this package's init runs after Lip Gloss's and before Bubble Tea's. +// Presetting the answer here makes that init a no-op; the two places that +// want the real answer, the terminal app's auto theme and `zn read +// --pretty`, call Detect and ask the terminal deliberately. +package termbg + +import ( + "os" + "strconv" + "strings" + "sync" + + "github.com/charmbracelet/lipgloss" + "github.com/muesli/termenv" +) + +func init() { + lipgloss.SetHasDarkBackground(Guess(os.Getenv("COLORFGBG"))) +} + +// Guess is the answer without asking the terminal: what COLORFGBG says +// (rxvt, Konsole and others export it as "fg;bg"), else dark, the same +// fallback termenv uses when a query goes unanswered. +func Guess(colorFGBG string) bool { + parts := strings.Split(strings.TrimSpace(colorFGBG), ";") + if len(parts) < 2 { + return true + } + bg, err := strconv.Atoi(parts[len(parts)-1]) + if err != nil || bg < 0 || bg > 15 { + return true + } + _, _, lightness := termenv.ConvertToRGB(termenv.ANSIColor(bg)).Hsl() + return lightness < 0.5 +} + +var ( + detectOnce sync.Once + detected bool +) + +// Detect asks the terminal on stdout once and remembers the answer, also +// for Lip Gloss's default renderer. It must run before Bubble Tea owns the +// terminal; it is the one place zn pays for the query, and only when the +// caller needs the real background. +func Detect() bool { + detectOnce.Do(func() { + detected = lipgloss.NewRenderer(os.Stdout).HasDarkBackground() + lipgloss.SetHasDarkBackground(detected) + }) + return detected +} diff --git a/internal/termbg/termbg_linux_test.go b/internal/termbg/termbg_linux_test.go new file mode 100644 index 0000000..4ec58bf --- /dev/null +++ b/internal/termbg/termbg_linux_test.go @@ -0,0 +1,127 @@ +//go:build linux + +package termbg_test + +import ( + "bytes" + "fmt" + "os" + "os/exec" + "strings" + "sync" + "testing" + "time" + + "golang.org/x/sys/unix" +) + +// openPTY returns the master side and the slave side of a new pseudo +// terminal, so a child process sees a real terminal on its standard streams. +func openPTY(t *testing.T) (*os.File, *os.File) { + t.Helper() + master, err := os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + t.Skipf("no pty available: %v", err) + } + n, err := unix.IoctlGetInt(int(master.Fd()), unix.TIOCGPTN) + if err != nil { + t.Fatal(err) + } + if err := unix.IoctlSetPointerInt(int(master.Fd()), unix.TIOCSPTLCK, 0); err != nil { + t.Fatal(err) + } + slave, err := os.OpenFile(fmt.Sprintf("/dev/pts/%d", n), os.O_RDWR|unix.O_NOCTTY, 0) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { master.Close(); slave.Close() }) + return master, slave +} + +// runOnTerminal runs this test binary as a child on a pty and returns what it +// wrote. answer, when set, plays the terminal's part for background queries. +func runOnTerminal(t *testing.T, mode string, answer func(master *os.File, seen []byte)) (string, time.Duration) { + t.Helper() + master, slave := openPTY(t) + cmd := exec.Command(os.Args[0]) + // termenv treats any output as a non-terminal while CI is set, which + // would make both tests pass without exercising anything; the child is + // a user's terminal session, so it runs without the runner's variables. + for _, kv := range os.Environ() { + if k, _, _ := strings.Cut(kv, "="); k != "CI" && k != "GITHUB_ACTIONS" && k != "TERM" && k != "COLORFGBG" && k != "ZN_TERMBG_CHILD" { + cmd.Env = append(cmd.Env, kv) + } + } + cmd.Env = append(cmd.Env, "ZN_TERMBG_CHILD="+mode, "TERM=xterm-256color") + cmd.Stdin, cmd.Stdout, cmd.Stderr = slave, slave, slave + cmd.SysProcAttr = &unix.SysProcAttr{Setsid: true, Setctty: true, Ctty: 0} + start := time.Now() + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + slave.Close() + var out bytes.Buffer + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + buf := make([]byte, 4096) + for { + n, err := master.Read(buf) + if n > 0 { + out.Write(buf[:n]) + if answer != nil { + answer(master, out.Bytes()) + } + } + if err != nil { + return + } + } + }() + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + select { + case <-done: + case <-time.After(8 * time.Second): + _ = cmd.Process.Kill() + t.Fatalf("child did not exit; output so far: %q", out.String()) + } + elapsed := time.Since(start) + master.Close() + wg.Wait() + return out.String(), elapsed +} + +// A terminal that never answers OSC 11 must not hold every command for +// termenv's five-second timeout, and no query may be written at all. +func TestStartupDoesNotQueryTheTerminal(t *testing.T) { + out, elapsed := runOnTerminal(t, "startup", nil) + if !strings.Contains(out, "child-ok") { + t.Fatalf("child output: %q", out) + } + if strings.Contains(out, "\x1b]11;?") || strings.Contains(out, "\x1b[6n") { + t.Fatalf("startup still queries the terminal: %q", out) + } + if elapsed > 3*time.Second { + t.Fatalf("startup took %s on a terminal that does not answer background queries", elapsed) + } +} + +// Detect is the deliberate query: it asks and believes the terminal. +func TestDetectAsksTheTerminalOnce(t *testing.T) { + var replied sync.Once + out, _ := runOnTerminal(t, "detect", func(master *os.File, seen []byte) { + if bytes.Contains(seen, []byte("\x1b[6n")) { + replied.Do(func() { + _, _ = master.Write([]byte("\x1b]11;rgb:ffff/ffff/ffff\x1b\\\x1b[1;1R")) + }) + } + }) + if !strings.Contains(out, "light") { + t.Fatalf("Detect ignored the terminal's light background: %q", out) + } + if strings.Count(out, "\x1b]11;?") != 1 { + t.Fatalf("expected exactly one background query: %q", out) + } +} diff --git a/internal/termbg/termbg_test.go b/internal/termbg/termbg_test.go new file mode 100644 index 0000000..12a46e6 --- /dev/null +++ b/internal/termbg/termbg_test.go @@ -0,0 +1,44 @@ +package termbg_test + +import ( + "fmt" + "os" + "testing" + + // Linked on purpose: its init is the terminal query this package + // pre-empts, so the child processes below behave like zn does. + _ "github.com/charmbracelet/bubbletea" + "github.com/charmbracelet/lipgloss" + + "github.com/ZenNotes/tui/internal/termbg" +) + +// The test binary doubles as the child process the pty tests run: it must +// import Bubble Tea and termbg exactly like zn, and nothing else can. +func TestMain(m *testing.M) { + switch os.Getenv("ZN_TERMBG_CHILD") { + case "": + os.Exit(m.Run()) + case "startup": + fmt.Println("child-ok") + case "detect": + fmt.Println(map[bool]string{true: "dark", false: "light"}[termbg.Detect()]) + } + os.Exit(0) +} + +func TestGuessReadsCOLORFGBG(t *testing.T) { + for input, dark := range map[string]bool{"": true, "15;0": true, "0;15": false, "0;7": false, "7;default;0": true, "junk": true, "0;99": true} { + if got := termbg.Guess(input); got != dark { + t.Errorf("Guess(%q) = %v, want %v", input, got, dark) + } + } +} + +// After init, Lip Gloss has an explicit answer, so nothing that asks the +// default renderer queries the terminal. +func TestDefaultRendererNeedsNoQuery(t *testing.T) { + if got := lipgloss.HasDarkBackground(); got != termbg.Guess(os.Getenv("COLORFGBG")) { + t.Fatalf("default renderer reports %v, want the preset guess", got) + } +} diff --git a/internal/tui/commands_parity.go b/internal/tui/commands_parity.go index cd89ffb..03a4544 100644 --- a/internal/tui/commands_parity.go +++ b/internal/tui/commands_parity.go @@ -440,6 +440,10 @@ func (a *App) connectServerFlow(rawURL string) { token = target.AuthToken } finish := func(a *App, token string) { + if err := config.CredentialsProblem(); err != nil { + a.notifyError("not connecting: " + err.Error()) + return + } client := remote.NewClient(baseURL, token) if _, err := client.GetCurrentVault(a.ctx); err != nil { switch remote.StatusOf(err) { diff --git a/internal/tui/theme.go b/internal/tui/theme.go index 2a9d790..269dd93 100644 --- a/internal/tui/theme.go +++ b/internal/tui/theme.go @@ -7,6 +7,7 @@ import ( "github.com/charmbracelet/lipgloss" "github.com/ZenNotes/tui/internal/config" + "github.com/ZenNotes/tui/internal/termbg" "github.com/ZenNotes/tui/internal/themes" ) @@ -70,7 +71,7 @@ type Theme struct { // run before Bubble Tea takes over the terminal, so Run calls it once and // the answer is reused for `:theme system` later. func DetectDarkBackground() bool { - return lipgloss.HasDarkBackground() + return termbg.Detect() } // themeSelection is the theme the preferences ask for: the desktop's diff --git a/internal/vault/frontmatter_test.go b/internal/vault/frontmatter_test.go new file mode 100644 index 0000000..d7fd7e4 --- /dev/null +++ b/internal/vault/frontmatter_test.go @@ -0,0 +1,40 @@ +package vault + +import "testing" + +// An empty block is still a block: record pages without properties carry +// one, and treating it as body text made every re-mirror add another. +func TestFrontmatterRecognisesAnEmptyBlock(t *testing.T) { + cases := []struct { + body, block, rest string + ok bool + }{ + {"---\n---\n# Title\n\nbody\n", "", "# Title\n\nbody\n", true}, + {"---\r\n---\r\n# T\n", "", "# T\n", true}, + {"---\n---", "", "", true}, + {"---\ntitle: x\n---\n# T\n", "title: x", "# T\n", true}, + {"---\ntitle: x\ntags: [a]\n---\nbody", "title: x\ntags: [a]", "body", true}, + // A later horizontal rule is body, not the closing fence. + {"---\n---\n# T\n\n---\n\nmore\n", "", "# T\n\n---\n\nmore\n", true}, + {"---\n----- not a fence\ntitle: x\n---\nbody", "----- not a fence\ntitle: x", "body", true}, + {"# No frontmatter\n---\n---\n", "", "# No frontmatter\n---\n---\n", false}, + {"---\nunterminated\n", "", "---\nunterminated\n", false}, + {"", "", "", false}, + } + for _, c := range cases { + block, rest, ok := Frontmatter(c.body) + if ok != c.ok || block != c.block || rest != c.rest { + t.Errorf("Frontmatter(%q) = (%q, %q, %v), want (%q, %q, %v)", c.body, block, rest, ok, c.block, c.rest, c.ok) + } + } +} + +func TestPrependKeepsAnEmptyFrontmatterBlockOnTop(t *testing.T) { + got := PrependToBody("---\n---\n# T\n", "new line") + if got != "---\n---\nnew line\n\n# T\n" { + t.Fatalf("prepend: %q", got) + } + if excerpt := BuildExcerpt("---\n---\n# Real title\n\nThe body.\n"); excerpt != "Real title The body." { + t.Fatalf("excerpt after empty block: %q", excerpt) + } +} diff --git a/internal/vault/parse.go b/internal/vault/parse.go index 7995f32..e5d0891 100644 --- a/internal/vault/parse.go +++ b/internal/vault/parse.go @@ -9,13 +9,17 @@ import ( // Regexes mirror the extractors in the desktop's vault-ops.ts and the // server's parse.go so the extracted metadata matches those builds. var ( - fenceLineRe = regexp.MustCompile("^[ \t]*(`{3,}|~{3,})(.*)$") - inlineCodeRe = regexp.MustCompile("`[^`\n]*`") - tagRe = regexp.MustCompile(`(?:^|\s)#(\p{L}[\p{L}\d_/-]*)`) - wikilinkRe = regexp.MustCompile(`(!?)\[\[([^\]|]+?)(?:\|[^\]]+)?\]\]`) - linkRe = regexp.MustCompile(`(!?)\[[^\]]*\]\(([^)\s]+)(?:\s+"[^"]*")?\)`) - embedRe = regexp.MustCompile(`!\[\[([^\]|]+?)(?:\|[^\]]+)?\]\]`) - frontmatterRe = regexp.MustCompile(`(?s)\A---\r?\n(.*?)\r?\n---\r?\n?`) + fenceLineRe = regexp.MustCompile("^[ \t]*(`{3,}|~{3,})(.*)$") + inlineCodeRe = regexp.MustCompile("`[^`\n]*`") + tagRe = regexp.MustCompile(`(?:^|\s)#(\p{L}[\p{L}\d_/-]*)`) + wikilinkRe = regexp.MustCompile(`(!?)\[\[([^\]|]+?)(?:\|[^\]]+)?\]\]`) + linkRe = regexp.MustCompile(`(!?)\[[^\]]*\]\(([^)\s]+)(?:\s+"[^"]*")?\)`) + embedRe = regexp.MustCompile(`!\[\[([^\]|]+?)(?:\|[^\]]+)?\]\]`) + // A block may be empty (`---\n---`): record pages without properties + // are written that way, and failing to see such a block made every + // re-mirror prepend another one. The empty form is tried first so a + // body's later horizontal rule cannot be mistaken for the closing fence. + frontmatterRe = regexp.MustCompile(`(?s)\A---\r?\n(?:---(?:\r?\n|\z)|(.*?)\r?\n---\r?\n?)`) headingRe = regexp.MustCompile(`(?m)^#{1,6}\s+`) imageMdRe = regexp.MustCompile(`!\[[^\]]*\]\([^)]*\)`) mdLinkRe = regexp.MustCompile(`\[([^\]]+)\]\([^)]*\)`) @@ -80,7 +84,10 @@ func Frontmatter(body string) (block string, rest string, ok bool) { if m == nil { return "", body, false } - return body[m[2]:m[3]], body[m[1]:], true + if m[2] >= 0 { + block = body[m[2]:m[3]] + } + return block, body[m[1]:], true } // ParseFrontmatterFields parses a frontmatter block into flat fields: diff --git a/internal/vault/tasks.go b/internal/vault/tasks.go index 54b0ef7..94f63e2 100644 --- a/internal/vault/tasks.go +++ b/internal/vault/tasks.go @@ -771,10 +771,9 @@ func RemoveTaskLine(markdown string, taskIndex int) (line string, body string, o } var ( - tasksHeadingRe = regexp.MustCompile(`(?i)^ {0,3}(#{1,6})\s+Tasks\s*$`) - anyHeadingRe = regexp.MustCompile(`^ {0,3}(#{1,6})\s+`) - thematicBreakRe = regexp.MustCompile(`^ {0,3}(?:(?:-[ \t]*){3,}|(?:\*[ \t]*){3,}|(?:_[ \t]*){3,})$`) - frontmatterOnlyR = regexp.MustCompile(`(?s)\A---\n.*?\n---\n?`) + tasksHeadingRe = regexp.MustCompile(`(?i)^ {0,3}(#{1,6})\s+Tasks\s*$`) + anyHeadingRe = regexp.MustCompile(`^ {0,3}(#{1,6})\s+`) + thematicBreakRe = regexp.MustCompile(`^ {0,3}(?:(?:-[ \t]*){3,}|(?:\*[ \t]*){3,}|(?:_[ \t]*){3,})$`) ) // InsertTasksUnderTasksHeading places task lines at the end of a `# Tasks` diff --git a/internal/vault/vault.go b/internal/vault/vault.go index 01549db..bd349c7 100644 --- a/internal/vault/vault.go +++ b/internal/vault/vault.go @@ -812,7 +812,11 @@ func (v *Vault) DeleteNote(rel string) error { if err != nil { return err } - if err := os.Remove(abs); err != nil && !errors.Is(err, os.ErrNotExist) { + // A server answers 404 here; saying "Deleted" for a note that was never + // there hid typos in scripts, so the local vault reports it too. + if err := os.Remove(abs); errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("Note not found: %s", v.relPosix(abs)) + } else if err != nil { return err } v.invalidateLayout() diff --git a/internal/vault/vault_test.go b/internal/vault/vault_test.go index 733f2ae..dbea22f 100644 --- a/internal/vault/vault_test.go +++ b/internal/vault/vault_test.go @@ -429,3 +429,23 @@ func TestCLIDialect(t *testing.T) { t.Fatalf("cli toggle #1 should reopen 'two': %q", next) } } + +// Deleting a note that is not there is an error, as it is on a server; +// "Deleted" for a typo hid the mistake from scripts. +func TestDeleteNoteReportsAMissingNote(t *testing.T) { + v := newTestVault(t) + writeFile(t, filepath.Join(v.Root(), "inbox", "Real.md"), "# Real\n") + if err := v.DeleteNote("inbox/Real.md"); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(filepath.Join(v.Root(), "inbox", "Real.md")); !os.IsNotExist(err) { + t.Fatal("note still exists") + } + err := v.DeleteNote("inbox/Real.md") + if err == nil || !strings.Contains(err.Error(), "Note not found: inbox/Real.md") { + t.Fatalf("second delete: %v", err) + } + if err := v.DeleteNote("inbox/never.md"); err == nil { + t.Fatal("deleting a note that never existed must fail") + } +}