Skip to content

Commit a447412

Browse files
zvasilevclaude
andcommitted
Improve DomBuilder/Viewer: unify links, fix HardBreak, remove dead code
- Extract register_link() helper to unify link reflect/focus logic between build_link (standalone) and collect_inline_words (inline) - Fix HardBreak rendering: split paragraphs at HardBreak boundaries into separate flexbox rows instead of emitting literal "\n" - Use string_view in build_code_block to avoid full-string copy - Delete move constructors on Viewer/Editor (prevent dangling refs) - Add Viewer cache invalidation for builder config changes - Remove dead StrongEmphasis enum value (cmark never produces it) - Document component() lifecycle contract in both headers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c09ef84 commit a447412

8 files changed

Lines changed: 108 additions & 40 deletions

File tree

docs/api-reference.md

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ enum class NodeType {
1818
Text, // Leaf text content
1919
Emphasis, // Italic (*text* or _text_)
2020
Strong, // Bold (**text** or __text__)
21-
StrongEmphasis, // Bold + italic (***text***)
2221
Link, // Hyperlink [text](url)
2322
ListItem, // Single list item
2423
BulletList, // Unordered list container
@@ -480,7 +479,6 @@ for (auto const& link : builder.link_targets()) {
480479
| Heading (level 3+) | `theme.heading3` decorator (default: bold + dim) |
481480
| Strong | `ftxui::bold` |
482481
| Emphasis | `ftxui::italic` (where supported) |
483-
| StrongEmphasis | `ftxui::bold` (italic not always combined) |
484482
| Link | `ftxui::underlined` + `theme.link` + `ftxui::reflect()` for boxes |
485483
| Link (focused) | adds `ftxui::inverted` + `ftxui::focus` |
486484
| BulletList | `" * "` prefix per item, indentation for nesting |

docs/architecture.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ A recursive tree structure representing parsed Markdown. Each node has a `NodeTy
9494
└── Text ("item two")
9595
```
9696

97-
Supported node types (18 total): Document, Heading, Paragraph, Text, Emphasis, Strong, StrongEmphasis, Link, ListItem, BulletList, OrderedList, CodeInline, CodeBlock, BlockQuote, SoftBreak, HardBreak, ThematicBreak, Image.
97+
Supported node types (17 total): Document, Heading, Paragraph, Text, Emphasis, Strong, Link, ListItem, BulletList, OrderedList, CodeInline, CodeBlock, BlockQuote, SoftBreak, HardBreak, ThematicBreak, Image.
9898

9999
### DomBuilder (`dom_builder.hpp`)
100100

docs/testing.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ Tests are grouped in blocks within `main()`. Each block is independent and can b
9494
| `test_headings.cpp` | Headings H1-H6: correct `NodeType::Heading`, `level` field, bold/dim decorators in rendered output. |
9595
| `test_bold.cpp` | `**bold**` and `__bold__`: AST produces `NodeType::Strong`, rendered output contains bold text. |
9696
| `test_italic.cpp` | `*italic*` and `_italic_`: AST produces `NodeType::Emphasis`, rendered output contains italic text. |
97-
| `test_bold_italic.cpp` | `***bold italic***` and mixed nesting: AST produces `NodeType::StrongEmphasis` or nested Strong/Emphasis. |
97+
| `test_bold_italic.cpp` | `***bold italic***` and mixed nesting: AST produces nested Strong/Emphasis nodes. |
9898
| `test_links.cpp` | `[text](url)`: AST structure, URL extraction, link nesting with bold/italic, DomBuilder link_targets(). |
9999
| `test_lists.cpp` | `- item` unordered lists: AST structure, bullet prefix rendering, list item content. |
100100
| `test_code.cpp` | `` `inline code` ``: AST produces `NodeType::CodeInline`, inverted rendering in output. |

markdown/include/markdown/ast.hpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ enum class NodeType {
1212
Text,
1313
Emphasis,
1414
Strong,
15-
StrongEmphasis,
1615
Link,
1716
ListItem,
1817
BulletList,

markdown/include/markdown/editor.hpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,13 @@ namespace markdown {
1313
class Editor {
1414
public:
1515
explicit Editor();
16+
Editor(Editor&&) = delete;
17+
Editor& operator=(Editor&&) = delete;
1618

19+
/// Returns the FTXUI component. Created on first call, cached thereafter.
20+
/// Configure the object (set_content, set_theme, etc.) before OR after
21+
/// this call — the renderer reads live state each frame.
22+
/// However, do not move this object after calling component().
1723
ftxui::Component component();
1824

1925
std::string const& content() const;

markdown/include/markdown/viewer.hpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ struct ExternalFocusable {
2323
class Viewer {
2424
public:
2525
explicit Viewer(std::unique_ptr<MarkdownParser> parser);
26+
Viewer(Viewer&&) = delete;
27+
Viewer& operator=(Viewer&&) = delete;
2628

2729
void set_content(std::string_view markdown_text);
2830
void set_scroll(float ratio);
@@ -32,9 +34,16 @@ class Viewer {
3234
void set_theme(Theme const& theme) {
3335
if (_theme.name != theme.name) { _theme = theme; ++_theme_gen; }
3436
}
35-
void set_max_quote_depth(int d) { _builder.set_max_quote_depth(d); }
37+
void set_max_quote_depth(int d) {
38+
_builder.set_max_quote_depth(d);
39+
++_builder_gen;
40+
}
3641
int max_quote_depth() const { return _builder.max_quote_depth(); }
3742

43+
/// Returns the FTXUI component. Created on first call, cached thereafter.
44+
/// Configure the object (set_content, set_theme, on_link_click, etc.)
45+
/// before OR after this call — the renderer reads live state each frame.
46+
/// However, do not move this object after calling component().
3847
ftxui::Component component();
3948
bool active() const { return _active; }
4049
void set_active(bool a) { _active = a; }
@@ -79,6 +88,8 @@ class Viewer {
7988
Theme _theme{theme_default()};
8089
uint64_t _theme_gen = 0;
8190
uint64_t _built_theme_gen = 0;
91+
uint64_t _builder_gen = 0;
92+
uint64_t _built_builder_gen = 0;
8293
std::function<void(std::string const&, LinkEvent)> _link_callback;
8394
ftxui::Component _component;
8495
std::vector<ExternalFocusable> _externals;

markdown/src/dom_builder.cpp

Lines changed: 84 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "markdown/dom_builder.hpp"
22

3+
#include <string_view>
4+
35
#include <ftxui/dom/flexbox_config.hpp>
46

57
namespace markdown {
@@ -44,6 +46,22 @@ ftxui::Decorator link_style(bool is_focused, ftxui::Decorator base,
4446
return base | ftxui::underlined | theme.link;
4547
}
4648

49+
// Register a link: create a LinkTarget, wrap each element in elems[from..]
50+
// with reflect for click detection, and apply focus to the first element.
51+
void register_link(Links& links, ftxui::Elements& elems, size_t from,
52+
std::string const& url, bool is_focused) {
53+
links.emplace_back(LinkTarget{.url = url});
54+
auto& target = links.back();
55+
size_t count = elems.size() - from;
56+
target.boxes.resize(count);
57+
for (size_t i = from; i < elems.size(); ++i) {
58+
elems[i] = elems[i] | ftxui::reflect(target.boxes[i - from]);
59+
}
60+
if (is_focused && count > 0) {
61+
elems[from] = elems[from] | ftxui::focus;
62+
}
63+
}
64+
4765
ftxui::Element build_node(ASTNode const& node, int depth, int qd,
4866
Links& links, int focused_link,
4967
Theme const& theme);
@@ -107,7 +125,8 @@ void collect_inline_words(ASTNode const& node, int depth, int qd,
107125
break;
108126
}
109127
case NodeType::SoftBreak:
110-
break; // flexbox gap handles spacing
128+
case NodeType::HardBreak:
129+
break; // handled by build_wrapping_container (HardBreak splits rows)
111130
case NodeType::Strong:
112131
collect_inline_words(child, depth + 1, qd, words,
113132
style | ftxui::bold, links, focused_link,
@@ -121,23 +140,10 @@ void collect_inline_words(ASTNode const& node, int depth, int qd,
121140
case NodeType::Link: {
122141
bool is_focused = is_next_link_focused(links, focused_link);
123142
auto ls = link_style(is_focused, style, theme);
124-
// Collect link words with underline, then wrap in reflect
125143
size_t before = words.size();
126144
collect_inline_words(child, depth + 1, qd, words,
127145
ls, links, focused_link, theme);
128-
// Wrap every word of this link with reflect for click detection
129-
links.emplace_back(LinkTarget{.url = child.url});
130-
auto& target = links.back();
131-
size_t word_count = words.size() - before;
132-
target.boxes.resize(word_count);
133-
bool first_word = true;
134-
for (size_t i = before; i < words.size(); ++i) {
135-
words[i] = words[i] | ftxui::reflect(target.boxes[i - before]);
136-
if (is_focused && first_word) {
137-
words[i] = words[i] | ftxui::focus;
138-
first_word = false;
139-
}
140-
}
146+
register_link(links, words, before, child.url, is_focused);
141147
break;
142148
}
143149
case NodeType::CodeInline:
@@ -162,8 +168,25 @@ bool is_plain_text_paragraph(ASTNode const& node) {
162168
return true;
163169
}
164170

171+
// Check if a node contains any HardBreak children.
172+
bool has_hard_break(ASTNode const& node) {
173+
for (auto const& child : node.children) {
174+
if (child.type == NodeType::HardBreak) return true;
175+
}
176+
return false;
177+
}
178+
179+
// Build a flexbox row from a flat list of word elements.
180+
ftxui::Element words_to_element(ftxui::Elements& words) {
181+
static const auto wrap_config = ftxui::FlexboxConfig().SetGap(1, 0);
182+
if (words.empty()) return ftxui::text("");
183+
if (words.size() == 1) return std::move(words[0]);
184+
return ftxui::flexbox(std::move(words), wrap_config);
185+
}
186+
165187
// Wrapping version of build_inline_container for block-level paragraphs.
166188
// Splits all inline content into word-level flexbox items for line wrapping.
189+
// HardBreak nodes force a new line by splitting into separate flexbox rows.
167190
ftxui::Element build_wrapping_container(ASTNode const& node, int depth, int qd,
168191
Links& links, int focused_link,
169192
Theme const& theme) {
@@ -186,13 +209,41 @@ ftxui::Element build_wrapping_container(ASTNode const& node, int depth, int qd,
186209
return ftxui::paragraph(combined);
187210
}
188211

189-
static const auto wrap_config = ftxui::FlexboxConfig().SetGap(1, 0);
190-
ftxui::Elements words;
191-
collect_inline_words(node, depth, qd, words, ftxui::nothing, links,
192-
focused_link, theme);
193-
if (words.empty()) return ftxui::text("");
194-
if (words.size() == 1) return std::move(words[0]);
195-
return ftxui::flexbox(std::move(words), wrap_config);
212+
// If no hard breaks, single flexbox row (common case).
213+
if (!has_hard_break(node)) {
214+
ftxui::Elements words;
215+
collect_inline_words(node, depth, qd, words, ftxui::nothing, links,
216+
focused_link, theme);
217+
return words_to_element(words);
218+
}
219+
220+
// Split at HardBreak boundaries: each segment becomes its own row.
221+
// Build a temporary ASTNode per segment and collect words from it.
222+
ftxui::Elements rows;
223+
ASTNode segment{.type = node.type};
224+
auto flush_segment = [&] {
225+
if (segment.children.empty()) {
226+
rows.push_back(ftxui::text(""));
227+
return;
228+
}
229+
ftxui::Elements words;
230+
collect_inline_words(segment, depth, qd, words, ftxui::nothing,
231+
links, focused_link, theme);
232+
rows.push_back(words_to_element(words));
233+
segment.children.clear();
234+
};
235+
236+
for (auto const& child : node.children) {
237+
if (child.type == NodeType::HardBreak) {
238+
flush_segment();
239+
} else {
240+
segment.children.push_back(child);
241+
}
242+
}
243+
flush_segment();
244+
245+
if (rows.size() == 1) return std::move(rows[0]);
246+
return ftxui::vbox(std::move(rows));
196247
}
197248

198249
// Build a ListItem: first Paragraph gets bullet/number prefix,
@@ -261,10 +312,10 @@ ftxui::Element build_link(ASTNode const& node, int depth, int qd,
261312
auto el = build_inline_container(node, depth, qd, links, focused_link,
262313
theme)
263314
| link_style(is_focused, ftxui::nothing, theme);
264-
if (is_focused) el = el | ftxui::focus;
265-
links.emplace_back(LinkTarget{.url = node.url});
266-
links.back().boxes.emplace_back();
267-
return el | ftxui::reflect(links.back().boxes.back());
315+
ftxui::Elements elems;
316+
elems.push_back(std::move(el));
317+
register_link(links, elems, 0, node.url, is_focused);
318+
return std::move(elems[0]);
268319
}
269320

270321
ftxui::Element build_bullet_list(ASTNode const& node, int depth, int qd,
@@ -307,17 +358,18 @@ ftxui::Element build_blockquote(ASTNode const& node, int depth, int qd,
307358
}
308359

309360
ftxui::Element build_code_block(ASTNode const& node, Theme const& theme) {
310-
std::string code = node.text;
311-
if (!code.empty() && code.back() == '\n') code.pop_back();
361+
std::string_view code = node.text;
362+
if (!code.empty() && code.back() == '\n') code.remove_suffix(1);
312363
ftxui::Elements lines;
313364
size_t start = 0;
314365
while (start <= code.size()) {
315366
auto end = code.find('\n', start);
316-
if (end == std::string::npos) {
317-
lines.push_back(ftxui::text(code.substr(start)));
367+
if (end == std::string_view::npos) {
368+
lines.push_back(ftxui::text(std::string(code.substr(start))));
318369
break;
319370
}
320-
lines.push_back(ftxui::text(code.substr(start, end - start)));
371+
lines.push_back(
372+
ftxui::text(std::string(code.substr(start, end - start))));
321373
start = end + 1;
322374
}
323375
if (lines.empty()) lines.push_back(ftxui::text(""));
@@ -382,7 +434,7 @@ ftxui::Element build_node(ASTNode const& node, int depth, int qd,
382434
case NodeType::SoftBreak:
383435
return ftxui::text(" ");
384436
case NodeType::HardBreak:
385-
return ftxui::text("\n");
437+
return ftxui::text("");
386438
default:
387439
return ftxui::text(node.text);
388440
}

markdown/src/viewer.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,15 +231,17 @@ ftxui::Component Viewer::component() {
231231
}
232232
_focused_link = (_focus_index >= ext_n) ? _focus_index - ext_n : -1;
233233

234-
// Rebuild element when content, focused link, or theme changes
234+
// Rebuild element when content, focused link, theme, or builder config changes
235235
if (_parsed_gen != _built_gen ||
236236
_focused_link != _last_focused_link ||
237-
_theme_gen != _built_theme_gen) {
237+
_theme_gen != _built_theme_gen ||
238+
_builder_gen != _built_builder_gen) {
238239
_cached_element = _builder.build(_cached_ast, _focused_link,
239240
_theme);
240241
_built_gen = _parsed_gen;
241242
_last_focused_link = _focused_link;
242243
_built_theme_gen = _theme_gen;
244+
_built_builder_gen = _builder_gen;
243245
}
244246
auto el = _cached_element;
245247

0 commit comments

Comments
 (0)