Skip to content

feat(docs): improve markdown layout controls#636

Merged
steipete merged 1 commit into
mainfrom
feat/docs-markdown-layout-fixes
May 22, 2026
Merged

feat(docs): improve markdown layout controls#636
steipete merged 1 commit into
mainfrom
feat/docs-markdown-layout-fixes

Conversation

@steipete
Copy link
Copy Markdown
Collaborator

Summary

  • Drop all-whitespace Markdown table header rows for both local Docs markdown rendering and Drive markdown import, while leaving code-block examples untouched.
  • After Drive markdown import, rewrite same-document #heading-slug links to native Google Docs headingId links.
  • Add explicit Docs layout controls to docs write and docs page-layout: --page-width, --page-height, --margin-left, --margin-right, --margin-top, --margin-bottom.
  • Preserve existing behavior by not widening --pageless unless sizing flags are explicit, and by letting docs page-layout --page-width ... preserve the current page mode unless --layout is also supplied.

Fixes #629.
Fixes #630.
Fixes #632.
Fixes #633.

Notes

Verification

  • go test ./internal/cmd -run 'TestDocsWrite_InvalidLayout|TestNormalizeMarkdownTables|TestBuildUpdateDocumentStyleRequest_ZeroMarginAllowed'
  • go test ./internal/cmd -run 'TestParseMarkdown_EmptyHeader|TestNormalizeMarkdownTables|TestDocsWrite_MarkdownReplace|TestDocsPageLayoutCmd|TestDocsWrite_PageSize|TestDocsWriteUpdate_Pageless'
  • make ci
  • ./bin/gog docs write doc1 --text hello --page-width 8.5in --margin-left 0.5in --margin-right 0.5in --dry-run --json
  • ./bin/gog docs page-layout doc1 --page-width 960 --dry-run --json
  • ./bin/gog docs page-layout doc1 --layout pages --page-width 960 --dry-run --json
  • ./bin/gog auth list --check --json --no-input confirms live Google Docs testing is blocked by missing GOG_KEYRING_PASSWORD for the file keyring in this shell.
  • Autoreview: /Users/steipete/Projects/agent-scripts/skills/autoreview/scripts/autoreview --mode local --prompt ... -> clean, no accepted/actionable findings.

@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

Codex review: needs maintainer review before merge.

Latest ClawSweeper review: 2026-05-22 15:40 UTC / May 22, 2026, 11:40 AM ET.

Workflow note: Future ClawSweeper reviews update this same comment in place.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

Summary
This PR adds Docs page-size and margin flags, normalizes empty-header Markdown tables, rewrites same-document Markdown heading links after Drive import, updates docs, and adds focused command tests.

Reproducibility: not applicable. as a PR review; the linked problems have source-level and CLI repros in the PR body/context, but I did not run live Google Docs mutation commands in this read-only review.

PR rating
Overall: 🐚 platinum hermit
Proof: 🌊 off-meta tidepool
Patch quality: 🐚 platinum hermit
Summary: The patch is focused, tested, and consistent with current Docs command patterns, with residual confidence limited mainly by missing live API proof in this shell.

Rank-up moves:

  • A maintainer may optionally run one live Google Docs smoke for page sizing and heading-link navigation before landing.
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Real behavior proof
Not applicable: The external-contributor proof gate does not apply because the PR author is marked COLLABORATOR; the body still lists tests, dry-runs, CI, and the reason live Google Docs testing was blocked.

Risk before merge

  • The PR body reports unit tests, dry-runs, and full CI, but live Google Docs mutation proof for the new layout/link behavior was blocked by the local keyring environment.

Maintainer options:

  1. Decide the mitigation before merge
    Land the focused Docs markdown/layout implementation after normal maintainer review and any desired live Google Docs smoke proof, keeping the explicit sizing flags rather than changing pageless width by default.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge
No ClawSweeper repair lane is needed because this is an active collaborator-authored implementation PR with no discrete automated fix finding.

Security
Cleared: The diff is limited to Go command logic, docs, tests, and changelog entries; I found no new dependency, secret, CI, release, or supply-chain concern.

Review details

Best possible solution:

Land the focused Docs markdown/layout implementation after normal maintainer review and any desired live Google Docs smoke proof, keeping the explicit sizing flags rather than changing pageless width by default.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a PR review; the linked problems have source-level and CLI repros in the PR body/context, but I did not run live Google Docs mutation commands in this read-only review.

Is this the best way to solve the issue?

Yes, with the usual maintainer validation: explicit page-size/margin flags avoid silently widening existing pageless documents, and the Markdown fixes are narrowly scoped to Drive import/local rendering paths.

Label changes:

  • add P2: The PR fixes bounded Docs markdown/layout problems and adds CLI flags with limited blast radius, but it is not an urgent runtime regression.
  • add rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🌊 off-meta tidepool, patch quality is 🐚 platinum hermit, and The patch is focused, tested, and consistent with current Docs command patterns, with residual confidence limited mainly by missing live API proof in this shell.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply because the PR author is marked COLLABORATOR; the body still lists tests, dry-runs, CI, and the reason live Google Docs testing was blocked.

Label justifications:

  • P2: The PR fixes bounded Docs markdown/layout problems and adds CLI flags with limited blast radius, but it is not an urgent runtime regression.
  • rating: 🐚 platinum hermit: Current PR rating is 🐚 platinum hermit because proof is 🌊 off-meta tidepool, patch quality is 🐚 platinum hermit, and The patch is focused, tested, and consistent with current Docs command patterns, with residual confidence limited mainly by missing live API proof in this shell.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The external-contributor proof gate does not apply because the PR author is marked COLLABORATOR; the body still lists tests, dry-runs, CI, and the reason live Google Docs testing was blocked.

What I checked:

  • PR author handling: The GitHub context marks the PR author association as COLLABORATOR, so ClawSweeper should not auto-close it even if the patch looks mergeable.
  • Current main lacks requested layout flags: Current main's DocsWriteCmd has --pageless but no page-size or margin flag group, while DocsPageLayoutCmd only accepts --layout. (internal/cmd/docs_edit.go:35, 23e74af31471)
  • PR adds explicit layout controls: The PR head adds DocsLayoutFlags for page width, page height, and four margins, plus validation/conversion to Docs API dimensions with zero-margin ForceSendFields. (internal/cmd/docs_layout.go:13, f7f51c6117ae)
  • PR preserves page mode when only sizing flags are supplied: DocsPageLayoutCmd only sends a document mode when --layout was provided or when no sizing flags were requested, matching the PR's compatibility claim. (internal/cmd/docs_set_page_layout.go:35, f7f51c6117ae)
  • Markdown fixes are covered in the PR patch: The PR normalizes blank Markdown table headers before Drive import and adds a Docs API post-pass that maps #slug links to headingId links. (internal/cmd/docs_markdown_links.go:16, f7f51c6117ae)
  • Focused tests added: The PR adds httptest coverage for Drive markdown upload normalization and heading-link rewrite requests, plus layout request coverage for docs write/page-layout. (internal/cmd/docs_write_markdown_test.go:96, f7f51c6117ae)

Likely related people:

  • steipete: Current main history and GitHub commit history show repeated ownership of Docs editing, markdown formatting, dry-run behavior, and the PR branch itself beyond merely proposing this change. (role: recent area contributor; confidence: high; commits: 7c511d8dd731, 6867fe850c7b, b4d6f559c399; files: internal/cmd/docs_edit.go, internal/cmd/docs_markdown.go, docs/docs-editing.md)
  • sebsnyk: GitHub history shows recent merged work on Docs page-layout and markdown tab replacement, and the linked issues in this PR come from the same Docs workflow area. (role: recent feature contributor; confidence: high; commits: 1b8a750026df, 58b866e9ac55; files: internal/cmd/docs_set_page_layout.go, internal/cmd/docs_edit.go, docs/docs-editing.md)
  • chrischall: Recent Docs table, page-break, and heading-style commits touch adjacent layout/table/formatting behavior that overlaps the review surface. (role: adjacent Docs contributor; confidence: medium; commits: 3a1b15592f47, dff8bb00c37e, 9d61f2e10206; files: internal/cmd/docs_markdown.go, docs/docs-editing.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 23e74af31471.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7f51c6117

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +333 to +334
} else if marker == fenceMarker {
inFence = false
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track full fence length before leaving fenced code blocks

normalizeMarkdownTablesForDriveImport treats any line that starts with ``` or ~~~ as an opening/closing fence token, but it only records the 3-character marker and not the actual fence length. In Markdown, a block opened with four backticks can legally contain a line of three backticks as content, so this logic can exit inFence too early and then rewrite `| ... |` lines that are still inside the code block. That silently mutates code examples during `docs write --replace --markdown` instead of leaving fenced content untouched.

Useful? React with 👍 / 👎.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels May 22, 2026
@clawsweeper
Copy link
Copy Markdown

clawsweeper Bot commented May 22, 2026

ClawSweeper PR egg

✨ Hatched: 🌱 uncommon Sunspot Lint Imp

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.

Rarity: 🌱 uncommon.
Trait: hums during re-review.
Image traits: location CI tidepool; accessory proof snapshot camera; palette moonlit blue and soft silver; mood calm; pose nestled inside a glowing shell; shell matte ceramic shell; lighting soft studio lighting; background soft code-shaped tiles.
Share on X: post this hatch
Copy: My PR egg hatched a 🌱 uncommon Sunspot Lint Imp in ClawSweeper.

What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@steipete steipete merged commit b26e190 into main May 22, 2026
11 checks passed
@steipete steipete deleted the feat/docs-markdown-layout-fixes branch May 22, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

1 participant