Skip to content

Fixes #4955. Replace regex-based Markdown lowering with Markdig AST walker#4981

Closed
Copilot wants to merge 64 commits intodevelopfrom
copilot/md-pipeline-refactor
Closed

Fixes #4955. Replace regex-based Markdown lowering with Markdig AST walker#4981
Copilot wants to merge 64 commits intodevelopfrom
copilot/md-pipeline-refactor

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 16, 2026

The MarkdownPipeline property was a dead API — Markdig's AST was parsed and immediately discarded, with all rendering driven by regex on raw source text. This meant custom pipeline settings had no effect, and several Markdown constructs were broken or missing entirely.

Core Change

LowerFromSourceText() (4 static Regex patterns + line-by-line processing) replaced by LowerFromAst(MarkdownDocument) that walks Markdig's parsed AST:

  • WalkBlock() — dispatches all block types: HeadingBlock, ParagraphBlock, FencedCodeBlock/CodeBlock, ThematicBreakBlock, QuoteBlock, ListBlock, Table, HtmlBlock
  • WalkInlines() — dispatches all inline types: LiteralInline, EmphasisInline, CodeInline, LinkInline, AutolinkInline, LineBreakInline, HtmlEntityInline, with ContainerInline fallback for unknown types (e.g. PipeTableDelimiterInline)
  • Blank-line preservation via source line-number gap analysis between consecutive top-level blocks
  • Task list detection via TaskList inline (Markdig extension)
  • Table Option A: extract cell text from AST → feed existing MarkdownTable re-parse path

New Strikethrough Role

Added MarkdownStyleRole.Strikethrough (maps to TextStyle.Strikethrough) for EmphasisInline with DelimiterChar == '~' && DelimiterCount >= 2.

Newly Unlocked Behaviors

Feature Before After
MarkdownPipeline property Dead API Controls rendering
~~strikethrough~~ Literal tildes Strikethrough style
Setext headings (Title\n=====) Broken Anchor + heading
Indented code blocks (4 spaces) Broken MarkdownCodeBlock SubView
Autolinks (<https://...>) Broken Link role
HTML entities (&amp;) Literal Transcoded character
Escaped chars (\*) Broken Correct pass-through

Test Updates

  • 56 new tests in AstLoweringTests.cs (baseline coverage, formerly-skipped known-limitation tests, per-phase unit tests)
  • Invalid_Table_Lines_Rendered_As_Text: relaxed >= 3 line assertion — Markdig correctly parses pipe-lines without a separator row as a single paragraph, not 3 separate paragraphs

Pull Request checklist:

  • I've named my PR in the form of "Fixes #issue. Terse description."
  • My code follows the style guidelines of Terminal.Gui - if you use Visual Studio, hit CTRL-K-D to automatically reformat your files before committing.
  • My code follows the Terminal.Gui library design guidelines
  • I ran dotnet test before commit
  • I have made corresponding changes to the API documentation (using /// style comments)
  • My changes generate no new warnings
  • I have checked my code and corrected any poor grammar or misspellings
  • I conducted basic QA to assure all features are working
Original prompt

use a branch name of "md-pipeline-refactor"

[Chronological Review: 1. User asked to study ./plans directory and summarize attack approach 2. Agent read the ast-based-lowering.md plan document (large file, read in sections) 3. Agent provided a terse summary of the 7-phase plan 4. User asked about /delegate command error with "orgin" target 5. Agent fetched copilot CLI documentation 6. Agent explained /delegate doesn't take a target argument and noted the typo 7. User clarified they DO get a remote prompt, pick "origin", and get "Could not determine GitHub repository from remote 'origin'" error 8. Agent ran `git remote -v` to diagnose 9. Agent identified the SSH shorthand `tig:gui-cs/Terminal.Gui` as the problem and suggested fix

Recent Commands: git remote -v was the last command executed]

[Intent Mapping:

  1. "Study ./plans. Tersly summarize how you'd go about attacking it." - Review plan docs
  2. "the /delegate command is giving me an error when I tell it to target orgin. Why?" - Debug CLI issue
  3. Follow-up with actual error message about remote 'origin' parsing failure]

[Technical Inventory: Terminal.Gui project, Markdig AST-based markdown lowering plan, GitHub Copilot CLI /delegate command, Git remote configuration, SSH config aliases]

[Code Archaeology: plans/ast-based-lowering.md - 890-line plan document for refactoring Markdown view]

[Progress Assessment: Both tasks completed - plan summarized, /delegate error diagnosed]

[Context Validation: All critical info captured]

[Recent Commands Analysis: git remote -v showed origin uses SSH shorthand tig:gui-cs/Terminal.Gui which /delegate can't parse]

1. Conversation Overview: - Primary Objectives: (1) "Study ./plans. Tersly summarize how you'd go about attacking it." (2) Debug why `/delegate` command errors when targeting origin remote - Session Context: Two distinct tasks - first reviewing a large refactoring plan, then diagnosing a Copilot CLI issue - User Intent Evolution: Started with codebase planning, shifted to tooling troubleshooting
  1. Technical Foundation:

    • Project: Terminal.Gui (gui-cs/Terminal.Gui) - TUI framework for .NET
    • Working Directory: C:\Users\Tig\s\gui-cs\pr-4955-title\
    • Branch context: PR-4955 related to title/markdown work
    • Markdig 0.39.0: Markdown parsing library used in the project
    • GitHub Copilot CLI: User's development tool, /delegate command sends session to cloud agent to create PR
  2. Codebase Status:

    • plans/ast-based-lowering.md (890 lines, 37.5KB):
      • Purpose: Comprehensive refactoring plan for Markdown view's lowering pipeline
      • Current State: Plan document, no implementation started
      • Key Content: 7-phase plan (Phase 0-6) to replace regex-based markdown parsing with Markdig AST walking
      • Core Problem: MarkdownPipeline property is dead API — Markdig AST is parsed then discarded, regex re-parses raw text
      • Architecture: Replace LowerFromSourceText() with LowerFromAst(MarkdownDocument) while keeping downstream IntermediateBlockBuildRenderedLines() → SubView pipeline unchanged
      • Phase 0: Pre-work tests (baseline coverage + known-failure tests)
      • Phase 1: AST block walker (HeadingBlock, ParagraphBlock, ThematicBreakBlock, etc.)
      • Phase 2: AST inline walker (LiteralInline, EmphasisInline, CodeInline, LinkInline, etc.) + add Strikethrough to MarkdownStyleRole
      • Phase 3: Table handling (Option A: extract cell text from AST, feed existing TableData)
      • Phase 4: Code block handling (FencedCodeBlock + indented CodeBlock)
      • Phase 5: The real dragon - nested block flattening (QuoteBlock/ListBlock trees → prefix strings)
      • Phase 6: Wire up, delete regex code, un-skip should-fail tests
      • Files to modify: MarkdownView.Parsing.cs, Markdown.cs, test files
      • Files NOT modified: Layout, Drawing, SubView classes stay unchanged
  3. Problem Resolution:

    • Issue: /delegate command fails with "Could not determine GitHub repository from remote 'origin'"
    • Root Cause: Git remote origin uses SSH shorthand tig:gui-cs/Terminal.Gui (custom SSH config alias) instead of standard GitHub URL format
    • Solution Provided: git remote set-url origin git@github.com:gui-cs/Terminal.Gui.git
    • Git remotes found:
      • origintig:gui-cs/Terminal.Gui (SSH shorthand - THE PROBLEM)
      • YourRobotOverlordhttps://github.com/YourRobotOverlord/Terminal.Gui.git
      • bdisphttps://github.com/BDisp/Terminal.Gui.git
  4. Progress Tracking:

    • Completed: Plan review and terse summary delivered
    • Completed: /delegate error diagnosed and fix provided
    • Not Started: Actual implementation of the ast-based-lowering plan
    • User asked "Want me to start implementing?" at end of plan summary — no response yet
  5. Active Work State:

    • Current Focus: Diagnosed /delegate error, provided fix
    • Most recent exchange was about the git remote URL for...

Created from Copilot CLI via the copilot delegate command.

Copilot AI and others added 30 commits April 13, 2026 09:25
Refactored MarkdownView parsing logic to use early returns and explicit types, improving readability and aligning with code style guidelines. Enhanced code block fence detection, added XML doc comments, and made minor formatting adjustments for consistency. Simplified constructors and improved window creation in the Markdown scenario.
- ListView on left shows all .md files from docfx/docs/ (fetched from GitHub API)
- MarkdownView on right renders the selected document
- Async HTTP loading for both file list and doc content
- SpinnerView in status bar during loading operations
- Error handling shows errors in MarkdownView instead of crashing
- Link clicks displayed in status bar
- Plans doc with all review issues found in PR #4955

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Changed scenario metadata to "Deepdives" with updated description.
- Renamed docs API URL constant for naming consistency.
- SpinnerView: AutoSpin now set via Initialized event, not by default.
- MarkdownView: Attribute selection no longer depends on focus.
- Enabled vertical scrollbar in MarkdownView by default.
- Added null check to App.Invoke in SpinnerView to prevent exceptions.
Added a parameterized test to MarkdownViewTests that verifies stray or unclosed special characters in markdown input do not cause infinite loops. The test covers various edge cases and asserts that rendering always produces at least one line.
Refactored MarkdownView to use StyledSegment for style selection, enabling context-aware rendering for links, code, and list markers. Improved word wrapping to prefer word boundaries, with hard breaks as fallback. Updated code style for project conventions and set default SchemeName. Added comprehensive Copilot-generated unit tests covering word wrap, style rendering, and link handling. Performed minor cleanups and variable renaming for clarity.
Refactored IntermediateBlock and RenderedLine to use primary constructors and added IsCodeBlock property. Updated MarkdownView to render code block lines with a full-width dimmed background. Propagated IsCodeBlock through layout and parsing logic. Improved test style and added a test to verify code block background rendering. Applied code style fixes for consistency.
Code blocks in MarkdownView now display a "⧉" copy button, allowing users to copy code block contents with a mouse click. Introduced CodeBlockRegion and CopyButtonHitTarget types to track code block regions and clickable areas. Refactored layout, draw, and mouse event handling to support this feature. Updated ChatView and SingleTurnView to use MarkdownView for markdown rendering and code block copy support. Moved MarkdownStyleRole to its own file and added unit tests for code block copy functionality.
Copy buttons for code blocks are now actual Button SubViews, styled to match code block backgrounds and positioned at the end of each code block's first visible line. The old manual hit-testing and drawing logic was removed. The copy glyph is now provided by Glyphs.Copy. Thematic breaks are rendered using LineCanvas. Parsing and layout were updated to track and handle thematic breaks correctly.
Introduces MarkdownTable and TableData for parsing and rendering GitHub-style Markdown tables as SubViews with box-drawing borders, column alignment, and proper cell content. Updates MarkdownView parsing and layout to detect tables, manage SubViews, and skip table lines in main drawing. Adds cleanup logic for table SubViews and comprehensive unit tests for parsing, rendering, and edge cases. Refactors related test code for consistency.
Refactored MarkdownView and MarkdownTable to use a new stateless MarkdownInlineParser for all inline Markdown formatting (bold, italic, code, links, images). Introduced MarkdownAttributeHelper to ensure consistent mapping of MarkdownStyleRole to Attribute across views. MarkdownTable now supports inline formatting in cells. Removed redundant parsing and attribute logic from MarkdownView. Added comprehensive unit tests for MarkdownInlineParser and MarkdownTable inline rendering. Updated documentation to reflect the new architecture.
Implements word-wrapping for MarkdownTable cell content, expanding row heights as needed. Borders now use screen coordinates and are merged by the parent MarkdownView. Cell drawing is refactored to handle wrapped lines with correct alignment and padding. Column width calculation now strips markdown formatting for accurate sizing. Table height logic is updated to reflect wrapped rows. Adds unit tests for wrapping, border rendering, markdown stripping, and height calculation.
Clarified and enforced a strict early return/guard clause rule in all major documentation and validation guides. Added `.claude/rules/early-return.md` with detailed guidance and examples. Updated code in `Markdown.cs` and `MarkdownTable.cs` to follow this pattern. Added comprehensive tests for the new table column width algorithm in `MarkdownTableTests.cs`, all using early returns as prescribed.
Improves word-wrapping to avoid breaking inside unclosed parentheses, fixing awkward line breaks in Markdown links with parentheses. Sets default content width to 80, enables horizontal scroll bar, and updates content width only on viewport size changes. Adds tests for correct parsing and rendering of such links.
SetContentSize now always uses Viewport.Width for content width instead of the maximum of _maxLineWidth and Viewport.Width. This change prevents the content width from exceeding the viewport, which may affect horizontal scrolling and layout.
- Added LineAttribute to Line for custom rendering attributes
- Refactored Line.Style setter for early return
- Thematic breaks in MarkdownView now use Line subviews
- Removed direct canvas rendering for thematic breaks
- Managed lifecycle of thematic break views in MarkdownView
- Reset MarkdownView viewport on new content load
- Added and updated unit tests for new behaviors
- Updated spinner style and AutoSpin handling in Markdown scenario
- Set StatusBar AlignmentModes to IgnoreFirstOrLast
- Refactored Line to use auto-properties for Style and LineAttribute
- Thematic breaks in MarkdownView now inset (X=1, Width=Dim.Fill(1))
- Improved anchor link handling in MarkdownView mouse logic
- Adjusted MarkdownViewTests for new thematic break rendering
SyncCopyButtons now uses Pos.AnchorEnd() for button X position, simplifying layout logic. GenerateAnchorSlug now replaces each space with a hyphen, matching GitHub's behavior. Added a test for anchor slug generation with special characters.
Refactored link handling to use MarkdownLinkRegion, enabling keyboard and mouse navigation with active link highlighting. Added command bindings for link navigation and activation. MarkdownTable now recalculates layout on resize. Improved copy button focus behavior and cleaned up legacy code.
MarkdownView now prioritizes externally set content width for layout, enabling accurate text wrapping and table sizing when resized. Introduced _externalContentWidth and _inLayout to manage layout state and prevent recursion. Updated OnContentSizeChanged to handle width changes correctly. Enhanced tests to verify line count and anchor movement when content width changes. Minor scenario and comment clarifications included.
Replaced inline code block drawing with a new internal MarkdownCodeBlock SubView that handles its own background, content, and copy button. Removed CodeBlockRegion and related logic. Updated layout and cleanup for SubView management. Added a test to verify code block width matches content size.
- MarkdownCodeBlock and MarkdownTable are now public and implement IDesignable for standalone and design-time use.
- Added parameterless constructors and public properties (CodeLines, Data) for direct content assignment.
- Implemented EnableForDesign to provide sample content and auto-sizing in design tools.
- MarkdownView exposes DefaultMarkdownSample for consistent sample content.
- Added unit tests for new constructors, properties, and designable support.
- Introduced MarkdownTester scenario in UICatalog for live Markdown editing and preview.
- Updated doc comments and code style for consistency with project conventions.
- Fix WrapSegments infinite loop when grapheme wider than column width
- Move MarkdownTable layout from draw to OnSubViewLayout
- Set ContentSize after BuildRenderedLines for proper scrolling
- Add HasFocus check for active-link highlight (prevents color reversal without focus)
- Emit OSC8 hyperlinks in active-link drawing branch
- Reset active link index on focus loss via OnHasFocusChanged
- Remove diagnostic Logging.Trace calls and cleanup tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refactored `MarkdownCodeBlock` and `MarkdownTable` to use parameterless constructors and property setters for content, replacing constructor parameters. Removed the copy button subview in `MarkdownCodeBlock` and replaced it with a clickable glyph. Added `UpdateContentSize` to recalculate dimensions on content change. Updated all usages and tests to use the new initialization pattern. Removed `SuperViewRendersLineCanvas` from `MarkdownTable` and revised related documentation. Added and updated tests to verify new defaults and behaviors.
Added a session-learnings.md document capturing key architectural insights and behavioral details from the MarkdownView, MarkdownCodeBlock, and MarkdownTable implementations. Topics include DimAuto sizing, SetContentSize effects, shadow style handling, attribute management, draw order, SubView refactoring, command system usage, drawing architecture, constructor patterns, focus handling, layout/draw separation, test conventions, and remaining Dim.Auto rollout work. This serves as a reference for future development and maintenance.
tig and others added 17 commits April 16, 2026 14:38
Adds UseThemeBackground to Markdown and MarkdownTable for theme-based backgrounds. Updates TextMateSyntaxHighlighter to use accurate per-theme colors. Propagates background and theme selection to tables and code blocks. Enhances UI scenarios with theme pickers and toggles. Fixes MarkdownTable constructor NRE. Updates and adds tests for theme background logic. All code follows project style conventions.
Ensure MarkdownTable and thematic break lines fill viewport with theme background. Disable all adornments in MarkdownTable for custom drawing. Add tests verifying correct background rendering. Restore dropped field initializations and add CurrentThemeName to syntax highlighter.
- Ensure Markdown viewport resets to top after setting text, preventing code blocks from stealing focus and scrolling.
- Set MarkdownCodeBlock to not focusable and disable tab stops.
- Change DropDownList mouse binding to LeftButtonPressed for immediate activation.
- Make TextField autocomplete nullable and use null-conditional access.
- Update tests and add regressions for Markdown viewport position.
- UI tweaks: DropDownList theme selector not focusable, Shortcut uses Text.
- Inline Markdown viewer sets screen size and disables auto-quit for better scrollback output.
Refactored inline Markdown rendering to use the ANSI driver in non-interactive mode, auto-sizing output to content. Made View.App setter public. Simplified MarkdownCodeBlock.Text and removed redundant background fill. Commented out theme config, clarified README heading, and deleted an unused test class.
Refactored argument parsing to use System.CommandLine, replacing manual logic. Changed default mode to full-screen; added --print/-p for print mode and --theme/-t for syntax highlighting. Updated README and project file. Improved Markdown UI theming and code block rendering.
Adds a custom --help/-h action to mdv that renders the embedded README.md as formatted markdown in the terminal using the print pipeline. README.md is now embedded as a resource and read at runtime for help output. Introduces MarkdownHelpAction to override default help behavior. Updates command-line wiring, project file, and documentation to support and describe the new help option. Includes minor code style and refactoring for consistency.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/gui-cs/Terminal.Gui/sessions/ff07f3fa-dafc-4768-986a-9f42d70e3583

Co-authored-by: tig <585482+tig@users.noreply.github.com>
- Refactored UI layout to use explicit Thickness constructors in ThemeViewer and Themes scenarios.
- Changed ThemeViewer scheme ordering: built-in schemes first, then custom schemes alphabetically.
- Added AttributeViewer to Themes scenario for displaying attribute details.
- Enhanced default Markdown sample with more features and clarity.
- Introduced AttributeViewer class with nullable reference types enabled.
- Added ast-based-lowering.md: plan for AST-based Markdown lowering.
- Added scheme-naming-review.md: review and proposal for scheme naming and new Accent scheme.
Copilot AI and others added 3 commits April 16, 2026 21:18
…e-refactor

Co-authored-by: tig <585482+tig@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor pipeline logic for improved efficiency Fixes #4955. Replace regex-based Markdown lowering with Markdig AST walker Apr 16, 2026
Copilot finished work on behalf of tig April 16, 2026 21:50
Copilot AI requested a review from tig April 16, 2026 21:50
@tig tig closed this Apr 16, 2026
@tig tig deleted the copilot/md-pipeline-refactor branch May 8, 2026 03:11
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.

2 participants