Skip to content

Conversation

@mscolnick
Copy link
Contributor

@mscolnick mscolnick commented Oct 30, 2025

Reverts a change made in #6969 which breaks a lot of markdown.

Fixes #6997

@vercel
Copy link

vercel bot commented Oct 30, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Oct 30, 2025 1:44pm

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR simplifies the markdown formatting logic by removing the special handling for long single-line markdown cells that would break them across multiple lines when exceeding 80 characters.

  • Removed the 80-character line length check and multiline formatting for single-line markdown
  • Updated test expectations to reflect the new single-line format for multiline markdown content
  • Corrected offset calculations from 16 to 11 to match the new format

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/smart-cells/src/parsers/markdown-parser.ts Removed line-length-based formatting logic that would break long single-line markdown into multiple lines
packages/smart-cells/src/tests/markdown-parser.test.ts Updated test snapshots and offset expectations to match the simplified single-line format
Comments suppressed due to low confidence (2)

packages/smart-cells/src/parsers/markdown-parser.ts:121

  • The condition isOneLine checks if the code doesn't include \\n, but the removed logic handled multiline content (e.g., '# Markdown Title\n\nSome content here.') that gets formatted as single-line. This creates a semantic mismatch: the condition name suggests single-line content, but it now formats multiline markdown without newlines in the output. Consider renaming isOneLine to shouldUseSingleLineFormat or adding a comment explaining that this formats multiline content on a single line when not bounded by quotes.
    if (isOneLine && !boundedByQuote) {
      const start = `mo.md(${quotePrefix}"""`;
      const end = `""")`;
      return { code: start + escapedCode + end, offset: start.length };
    }

packages/smart-cells/src/parsers/markdown-parser.ts:121

  • This change creates an inconsistency with the Python backend. The referenced comment on line 100 states 'NB. Must be kept consistent with marimo/_convert/utils.py::markdown_to_marimo', but the Python construct_markdown_call function in marimo/_ast/codegen.py (lines 195-214) uses format_tuple_elements which respects the MAX_LINE_LENGTH = 80 constant. The removal of line-length checking in TypeScript breaks this documented consistency requirement.
    if (isOneLine && !boundedByQuote) {
      const start = `mo.md(${quotePrefix}"""`;
      const end = `""")`;
      return { code: start + escapedCode + end, offset: start.length };
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@mscolnick mscolnick merged commit 7a1e15d into main Oct 30, 2025
32 checks passed
@mscolnick mscolnick deleted the ms/markdown-single-lines branch October 30, 2025 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error when entering Markdown text in 0.17.3

2 participants