-
-
Notifications
You must be signed in to change notification settings - Fork 1
Implementing #28 Djot support #33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update migrates content parsing and rendering from Markdown to Djot format across the Cynthia Websites Mini Client and Server. It introduces a new Djot parser module, updates dependencies, and removes the previous Markdown rendering logic. Test suites and snapshots are expanded to validate Djot parsing, and the server now converts Markdown to Djot using Pandoc. Configuration files and example content are updated to reflect the new Djot-based workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant Pandoc
participant Client
participant DjotParser
User->>Server: Request content file
Server->>Server: Detect file extension
alt If Markdown file
Server->>Pandoc: Convert Markdown to Djot
Pandoc-->>Server: Return Djot content
else If Djot file
Server->>Server: Use Djot content directly
end
Server-->>Client: Send Djot content
Client->>DjotParser: Parse Djot to HTML
DjotParser-->>Client: Return HTML elements
Client->>User: Render HTML page
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
From what I got so far, either Jot's Djot support is limited compared to my earlier thoughts, or I'm implementing something wrong. |
|
Can't continue for now, Jot is not recognising enough of the spec yet. |
Update from branch 'main'
|
Also implements #4 as a side effect. |
|
Now that we've got decent support by implementing preprocessing, I think we're ready to PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (4)
mise.toml (1)
3-3: Consider pinning the full OTP version
erlang = "27"brings you onto the latest OTP major, but minor/runtime mismatches between local dev and CI/CD containers can be painful. Pinning to27.0(or the patch actually used in CI) avoids surprises while still being explicit.cynthia_websites_mini_client/src/cynthia_websites_mini_client/pageloader/postlistloader.gleam (1)
138-139: Remove magic string"descr.dj"– extract a single constantThe same literal appears twice; future renames will require duplicate edits and invite drift.
- [pottery.parse_html(post.description, "descr.dj")], + [pottery.parse_html(post.description, pottery.DESCRIPTION_FILE)], ... - [pottery.parse_html(page.description, "descr.dj")], + [pottery.parse_html(page.description, pottery.DESCRIPTION_FILE)],A small exported constant (or helper like
pottery.parse_description/1) keeps the call-site tidy and upgrade-proof.Also applies to: 189-190
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/djotparse.gleam (2)
757-786: Handle edge case in autolink preprocessingThe autolink preprocessing uses "🔗AUTOLINK🔗" as a marker, but doesn't handle the case where this string might already exist in the input. While unlikely, this could cause incorrect parsing.
Consider using a more unique marker or escaping existing occurrences first.
89-134: Complex ordered list detection in paragraphsThe code implements custom logic to detect and extract ordered lists from paragraph text. This suggests the Jot parser might not be handling ordered lists as expected.
Is this a known limitation of the Jot library? Consider documenting why this workaround is necessary, or file an issue with the Jot project if this is unexpected behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
cynthia_websites_mini_client/birdie_snapshots/autolinks_test.accepted(1 hunks)cynthia_websites_mini_client/birdie_snapshots/djot_with_preprocessing_test.accepted(1 hunks)cynthia_websites_mini_client/birdie_snapshots/simple_djot_test.accepted(1 hunks)cynthia_websites_mini_client/gleam.toml(1 hunks)cynthia_websites_mini_client/manifest.toml(4 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pageloader/postlistloader.gleam(2 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery.gleam(4 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/djotparse.gleam(1 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/markdown_renders_ffi.ts(0 hunks)cynthia_websites_mini_client/src/cynthia_websites_mini_client/view.gleam(2 hunks)cynthia_websites_mini_client/test/cynthia_websites_mini_client_test.gleam(2 hunks)cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam(11 hunks)mise.toml(1 hunks)package.json(0 hunks)
💤 Files with no reviewable changes (2)
- package.json
- cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/markdown_renders_ffi.ts
🔇 Additional comments (25)
cynthia_websites_mini_client/gleam.toml (1)
24-24: Verify Jot’s JS-target support locally before mergingWe attempted to run
gleam build --target javascriptin the sandbox, but the Gleam CLI isn’t installed here. Please run the following locally to confirm thatjotv5 cross-compiles cleanly (no missing FFI shims):
- File: cynthia_websites_mini_client/gleam.toml
- Lines: 24
jot = ">= 5.0.0 and < 6.0.0"cynthia_websites_mini_client/src/cynthia_websites_mini_client/view.gleam (1)
29-29: No remaining “notfound.md” literals detected
Ranrg -n '"notfound\.md"'with no matches—all fallback references are now consistently using “notfound.dj”.cynthia_websites_mini_client/birdie_snapshots/simple_djot_test.accepted (1)
1-26: No issues – snapshot looks consistent with the new Djot renderer.cynthia_websites_mini_client/birdie_snapshots/autolinks_test.accepted (1)
1-14: Test snapshot correctly validates autolinks functionality.The snapshot demonstrates proper autolink processing with correctly formatted HTML output, including appropriate CSS classes and accessible link text that matches the URL.
cynthia_websites_mini_client/manifest.toml (2)
8-8: Local package version bump looks significant.The
cynthia_websites_mini_sharedpackage version jumped from0.1.0to1.0.6, which suggests substantial changes in the shared module.Please verify that the shared package changes are compatible and properly tested, as this is a major version bump that could introduce breaking changes.
27-27: Dependency additions support Djot migration effectively.The addition of
jot(v5.0.0) andsplitter(v1.0.0) packages with appropriate version constraints properly supports the Djot parsing functionality being implemented.Also applies to: 36-36, 53-53
cynthia_websites_mini_client/birdie_snapshots/djot_with_preprocessing_test.accepted (2)
14-16: Task list rendering follows accessibility best practices.The task list items are properly rendered with disabled checkboxes and appropriate ARIA attributes, following accessibility guidelines for read-only content.
16-16: Blockquote styling maintains visual hierarchy.The blockquote is properly styled with border, padding, and background to create clear visual separation while maintaining readability.
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery.gleam (4)
4-4: Djot parser import added appropriately.The import of the
djotparsemodule supports the migration from Markdown to Djot parsing functionality.
35-35: File extension references updated consistently.All calls to
parse_htmlhave been updated from"descr.md"to"descr.dj", ensuring consistent handling of Djot files throughout the codebase.Also applies to: 51-51
134-135: Djot parsing architecture improves security and performance.The direct return of lustre elements from
djotparse.entry_to_conversion()eliminates the need forunsafe_raw_html, improving both security and performance compared to the previous Markdown approach.
134-135: Confirm Markdown Migration Before Dropping SupportThe updated pattern in pottery.gleam only matches Djot files (
.dj,.djot), removing Markdown extensions. Yet I still see Markdown references elsewhere:• cynthia_websites_mini_client/test/cynthia_websites_mini_client_test.gleam
– External URLs ending in.mdused in test fixtures
• cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam
–files.file_exist(.../README.md)checks
–string.ends_with(file, "markdown")and related conversion comments
• cynthia_websites_mini_client/src/.../github_layout.gleam
– “Main content styled like GitHub markdown content” commentPlease verify that:
- All existing Markdown content has been fully migrated to Djot
- Tests referencing
.mdURLs or server‐side Markdown logic have been updated (or are intentionally left unchanged)- No Markdown files remain in the content flow that would now be ignored
cynthia_websites_mini_client/test/cynthia_websites_mini_client_test.gleam (5)
1-3: Test imports properly support Djot functionality.The addition of imports for
birdie,djotparse,string,element, andhtmlprovides the necessary dependencies for comprehensive Djot testing.Also applies to: 6-7
19-25: Basic Djot parsing test provides solid foundation.The
simple_djot_testvalidates core Djot parsing functionality with a representative sample including headings, paragraphs, and lists.
27-33: Preprocessing test validates advanced Djot features.The test comprehensively validates task lists, blockquotes, and other preprocessed content, ensuring the Djot pipeline handles complex markup correctly.
35-41: Autolinks test validates critical URL handling.The test ensures that autolinks (URLs in angle brackets) are properly converted to HTML links, which is essential for content migration from Markdown.
43-60: Debug test validates preprocessing pipeline.The
debug_autolinks_testfunction provides valuable insight into the preprocessing step, specifically testing the autolink marker replacement logic.cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam (2)
19-19: Clean up importsThe imports have been reorganized to explicitly import
NoneandSomefromgleam/option. This improves code clarity.
919-966: Well-structured Djot examplesThe example Djot content comprehensively demonstrates various features including:
- Heading IDs (
{#hello-world})- Lists (numbered, bullet, and task lists)
- Images with alt text
- Blockquotes
- Code blocks with language specification
- Tables
This provides excellent documentation for users migrating to Djot.
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/djotparse.gleam (6)
18-22: Clear entry point for Djot conversionThe
entry_to_conversionfunction provides a clean API that handles preprocessing, parsing, and conversion in a well-structured pipeline.
24-49: Well-organized preprocessing pipelineThe preprocessing pipeline correctly handles various Djot extensions in a logical order:
- Normalize line endings and escape handling
- Process heading attributes (correctly done early)
- Handle complex structures (images, tables, quotes)
- Process inline elements (autolinks, task lists)
The order is appropriate for avoiding conflicts between transformations.
689-712: Thoughtful blockquote handlingThe blockquote preprocessing correctly:
- Groups consecutive quote lines
- Handles empty lines and line breaks
- Manages the backslash escape character
- Converts to properly styled HTML
The implementation handles edge cases well.
325-374: Proper handling of links with client-side routingThe link handling correctly:
- Transforms relative URLs for client-side routing
- Handles both direct URLs and references
- Provides error feedback for missing references
- Maintains proper styling with Tailwind classes
Good implementation of client-side navigation requirements.
478-516: Well-implemented footnotes sectionThe footnotes implementation includes:
- Conditional rendering (only when footnotes exist)
- Proper semantic HTML with ordered list
- Accessibility attributes (
role="doc-backlink")- Bidirectional linking between references and footnotes
Excellent attention to detail.
897-959: Smart heading attribute preprocessingThe preprocessing correctly handles the pattern where heading IDs are specified on a separate line before the heading (common in some Markdown flavors) and converts them to inline Djot format. This improves compatibility with existing content.
| to: "themes.dj", | ||
| // We are downloading markdown content as Djot content without conversion... Hopefully it'll parse correctly. | ||
| // Until the documentation is updated to reflect the new default file type :) | ||
| from: "https://raw.githubusercontent.com/CynthiaWebsiteEngine/Mini-docs/refs/heads/main/content/3.%20Customisation/3.2-themes.md", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential compatibility issue with external Markdown content
The comment indicates that Markdown content is being downloaded and used as Djot content without conversion. This is risky because Markdown and Djot have syntax differences that could cause parsing errors.
Since you've already implemented Pandoc conversion for local files, consider applying the same conversion to external content to ensure compatibility.
🤖 Prompt for AI Agents
In cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam
around lines 896 to 899, the code downloads external Markdown content and treats
it as Djot without conversion, risking parsing errors due to syntax differences.
To fix this, apply the existing Pandoc conversion process used for local files
to this external content after downloading it, ensuring the content is properly
converted from Markdown to Djot format before further processing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not want to utilise pandoc convertions on first set up, since this ruins the out of the box experience for those without pandoc preinstalled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@strawmelonjuice, that's a perfectly reasonable design choice! You're right that requiring Pandoc during initial setup would create a barrier for users who just want to get started quickly. The out-of-the-box experience is important for a "mini" engine.
The comment in the code does acknowledge this trade-off ("Hopefully it'll parse correctly"), and for initial setup content, the slight risk of parsing differences is acceptable compared to the complexity of requiring external dependencies.
✏️ Learnings added
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| // Now, conversion to Djot for markdown files done in-place: | ||
| let converted: Result(#(String, String), String) = case | ||
| string.ends_with(file, "markdown") | ||
| |> bool.or( | ||
| string.ends_with(file, "md") |> bool.or(string.ends_with(file, "mdown")), | ||
| ) | ||
| { | ||
| True -> { | ||
| // If the file is external, we need to write it to a temporary file first. | ||
| let wri = case possibly_extern { | ||
| Some(..) -> { | ||
| simplifile.write(file, inner_plain) | ||
| |> result.replace_error( | ||
| "There was an error while writing the external content to '" | ||
| <> file |> premixed.text_bright_yellow() | ||
| <> "'.", | ||
| ) | ||
| } | ||
| None -> Ok(Nil) | ||
| } | ||
| use _ <- result.try(wri) | ||
|
|
||
| use pandoc_path <- result.try(result.replace_error( | ||
| bun.which("pandoc"), | ||
| "There is a markdown file in Cynthia's content folder, but to convert that to Djot and display it, you need to have Pandoc installed on the PATH, which it is not!", | ||
| )) | ||
| let pandoc_child = | ||
| spawn.sync(spawn.OptionsToSubprocess( | ||
| [pandoc_path, file, "-f", "gfm", "-t", "djot"], | ||
| cwd: Some(process.cwd()), | ||
| env: None, | ||
| stderr: Some(spawn.Pipe), | ||
| stdout: Some(spawn.Pipe), | ||
| )) | ||
| let pandoc_child = case | ||
| { | ||
| let assert spawn.SyncSubprocess(asserted_sync_child) = pandoc_child | ||
| spawn.success(asserted_sync_child) | ||
| } | ||
| { | ||
| True -> Ok(pandoc_child) | ||
| False -> { | ||
| Error( | ||
| "There was an error while trying to convert '" | ||
| <> file |> premixed.text_bright_yellow() | ||
| <> "' to Djot: \n" | ||
| <> result.unwrap(spawn.stderr(pandoc_child), "") | ||
| <> "\n\nMake sure you have at least Pandoc 3.7.0 installed on your system, earlier versions may not work correctly.", | ||
| ) | ||
| } | ||
| } | ||
| use pandoc_child <- result.try(pandoc_child) | ||
| let new_inner_plain: Result(String, String) = | ||
| spawn.stdout(pandoc_child) | ||
| |> result.replace_error("") | ||
| use new_inner_plain <- result.try(new_inner_plain) | ||
|
|
||
| // If the file was external, we need delete the temporary file. | ||
| let re = case possibly_extern { | ||
| Some(..) -> { | ||
| simplifile.delete(file) | ||
| |> result.replace_error( | ||
| "There was an error while deleting the temporary file '" | ||
| <> file |> premixed.text_bright_yellow() | ||
| <> "'.", | ||
| ) | ||
| } | ||
| None -> Ok(Nil) | ||
| } | ||
|
|
||
| use _ <- result.try(re) | ||
|
|
||
| Ok(#(new_inner_plain, file <> ".dj")) | ||
| } | ||
|
|
||
| False -> { | ||
| Ok(#(inner_plain, file)) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Review the Pandoc conversion implementation
The implementation correctly handles Markdown to Djot conversion using Pandoc. However, consider these points:
-
Version requirement: The error message mentions Pandoc 3.7.0 is required. This should be documented in the project's README.
-
Subprocess execution: Using
spawn.syncblocks the event loop. For better performance in a server environment, consider using async subprocess execution. -
Temporary file handling: The temporary file cleanup (lines 615-627) could fail to execute if an error occurs between file creation and deletion. Consider using a more robust approach like a try-finally pattern or a temporary directory that's cleaned up automatically.
Let me verify if Pandoc is commonly available and check the current latest version:
🌐 Web query:
What is the latest version of Pandoc and when was Djot support added to Pandoc?
💡 Result:
The latest version of Pandoc as of July 2025 is 3.7, released on May 15, 2025[1][6].
Djot support was added to Pandoc as both an input and an output format in version 3.1.12. This was confirmed by the Pandoc maintainer in April 2024, who stated that Djot support was already available in the current release at that time[8].
Citations:
- 1: https://github.com/jgm/pandoc/releases
- 2: https://djot.net
- 3: http://boisgera.github.io/pandoc/changelog/
- 4: https://pandoc.org/installing.html
- 5: https://github.com/jgm/djot
- 6: https://tracker.debian.org/pandoc
- 7: https://pandoc.org
- 8: Pandoc now supports djot jgm/djot#270
- 9: https://news.ycombinator.com/item?id=33867636
- 10: https://pypi.org/project/pypandoc/
Improve Pandoc conversion docs, performance, and cleanup
Please address the following points to harden the Markdown-to-Djot conversion logic:
- Document minimum Pandoc version
– Add to README that Djot support was introduced in Pandoc 3.1.12 (April 2024) and that we recommend using Pandoc ≥ 3.7.0 for the latest fixes. - Update the runtime error message
– Reflect the fact that Djot support exists as of 3.1.12 and note the recommendation for ≥3.7.0. - Use non-blocking subprocesses
– Replacespawn.syncwith your platform’s async subprocess API (or a task/future) to avoid blocking the event loop. - Guarantee temp-file cleanup
– Encapsulate the write/convert/delete sequence in a try-finally pattern or use a temporary-directory helper so files are always removed, even on errors.
Suggested diff for the error message:
- "Make sure you have at least Pandoc 3.7.0 installed on your system, earlier versions may not work correctly."
+ "Make sure you have Pandoc ≥ 3.1.12 (Djot support) — we recommend ≥ 3.7.0 for current releases."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Now, conversion to Djot for markdown files done in-place: | |
| let converted: Result(#(String, String), String) = case | |
| string.ends_with(file, "markdown") | |
| |> bool.or( | |
| string.ends_with(file, "md") |> bool.or(string.ends_with(file, "mdown")), | |
| ) | |
| { | |
| True -> { | |
| // If the file is external, we need to write it to a temporary file first. | |
| let wri = case possibly_extern { | |
| Some(..) -> { | |
| simplifile.write(file, inner_plain) | |
| |> result.replace_error( | |
| "There was an error while writing the external content to '" | |
| <> file |> premixed.text_bright_yellow() | |
| <> "'.", | |
| ) | |
| } | |
| None -> Ok(Nil) | |
| } | |
| use _ <- result.try(wri) | |
| use pandoc_path <- result.try(result.replace_error( | |
| bun.which("pandoc"), | |
| "There is a markdown file in Cynthia's content folder, but to convert that to Djot and display it, you need to have Pandoc installed on the PATH, which it is not!", | |
| )) | |
| let pandoc_child = | |
| spawn.sync(spawn.OptionsToSubprocess( | |
| [pandoc_path, file, "-f", "gfm", "-t", "djot"], | |
| cwd: Some(process.cwd()), | |
| env: None, | |
| stderr: Some(spawn.Pipe), | |
| stdout: Some(spawn.Pipe), | |
| )) | |
| let pandoc_child = case | |
| { | |
| let assert spawn.SyncSubprocess(asserted_sync_child) = pandoc_child | |
| spawn.success(asserted_sync_child) | |
| } | |
| { | |
| True -> Ok(pandoc_child) | |
| False -> { | |
| Error( | |
| "There was an error while trying to convert '" | |
| <> file |> premixed.text_bright_yellow() | |
| <> "' to Djot: \n" | |
| <> result.unwrap(spawn.stderr(pandoc_child), "") | |
| <> "\n\nMake sure you have at least Pandoc 3.7.0 installed on your system, earlier versions may not work correctly.", | |
| ) | |
| } | |
| } | |
| use pandoc_child <- result.try(pandoc_child) | |
| let new_inner_plain: Result(String, String) = | |
| spawn.stdout(pandoc_child) | |
| |> result.replace_error("") | |
| use new_inner_plain <- result.try(new_inner_plain) | |
| // If the file was external, we need delete the temporary file. | |
| let re = case possibly_extern { | |
| Some(..) -> { | |
| simplifile.delete(file) | |
| |> result.replace_error( | |
| "There was an error while deleting the temporary file '" | |
| <> file |> premixed.text_bright_yellow() | |
| <> "'.", | |
| ) | |
| } | |
| None -> Ok(Nil) | |
| } | |
| use _ <- result.try(re) | |
| Ok(#(new_inner_plain, file <> ".dj")) | |
| } | |
| False -> { | |
| Ok(#(inner_plain, file)) | |
| } | |
| } | |
| // ... | |
| let pandoc_child = case | |
| { | |
| let assert spawn.SyncSubprocess(asserted_sync_child) = pandoc_child | |
| spawn.success(asserted_sync_child) | |
| } | |
| { | |
| True -> Ok(pandoc_child) | |
| False -> { | |
| Error( | |
| "There was an error while trying to convert '" | |
| <> file |> premixed.text_bright_yellow() | |
| <> "' to Djot: \n" | |
| <> result.unwrap(spawn.stderr(pandoc_child), "") | |
| - <> "\n\nMake sure you have at least Pandoc 3.7.0 installed on your system, earlier versions may not work correctly.", | |
| + <> "\n\nMake sure you have Pandoc ≥ 3.1.12 (Djot support) — we recommend ≥ 3.7.0 for current releases.", | |
| ) | |
| } | |
| } | |
| // ... |
🤖 Prompt for AI Agents
In cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam
around lines 557 to 636, improve the Markdown-to-Djot conversion by updating the
README to document that Djot support was introduced in Pandoc 3.1.12 and
recommend using Pandoc ≥ 3.7.0. Modify the runtime error message to reflect this
version information. Replace the blocking spawn.sync call with an asynchronous
subprocess API or task/future to prevent blocking the event loop. Finally,
ensure the temporary file created for external markdown files is always deleted
by wrapping the write, convert, and delete steps in a try-finally pattern or
using a temporary-directory helper to guarantee cleanup even if errors occur.
| fn preprocess_task_lists(djot: String) -> String { | ||
| djot | ||
| |> string.split("\n") | ||
| |> list.map(fn(line) { | ||
| let trimmed = string.trim(line) | ||
| case string.starts_with(trimmed, "- [ ] ") { | ||
| True -> { | ||
| let content = string.drop_start(trimmed, 6) | ||
| { | ||
| html.div([attribute.class("flex items-center mb-2")], [ | ||
| html.input([ | ||
| attribute.type_("checkbox"), | ||
| attribute.disabled(True), | ||
| attribute.class("mr-2 accent-primary"), | ||
| ]), | ||
| element.text(content), | ||
| ]) | ||
| } | ||
| |> element_to_raw_djotstring | ||
| } | ||
| False -> | ||
| case | ||
| string.starts_with(trimmed, "- [x] ") | ||
| || string.starts_with(trimmed, "- [X] ") | ||
| { | ||
| True -> { | ||
| let content = string.drop_start(trimmed, 6) | ||
| "```=html\n<div class=\"flex items-center mb-2\"><input type=\"checkbox\" checked disabled class=\"mr-2 accent-primary\">" | ||
| <> content | ||
| <> "</div>\n```" | ||
| } | ||
| False -> line | ||
| } | ||
| } | ||
| }) | ||
| |> string.join("\n") | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Inconsistent HTML generation in task lists
The unchecked task items use Lustre element building (lines 728-737) while checked items use manual HTML string building (lines 746-748). For consistency and maintainability, use the same approach for both.
True -> {
let content = string.drop_start(trimmed, 6)
- "```=html\n<div class=\"flex items-center mb-2\"><input type=\"checkbox\" checked disabled class=\"mr-2 accent-primary\">"
- <> content
- <> "</div>\n```"
+ {
+ html.div([attribute.class("flex items-center mb-2")], [
+ html.input([
+ attribute.type_("checkbox"),
+ attribute.checked(True),
+ attribute.disabled(True),
+ attribute.class("mr-2 accent-primary"),
+ ]),
+ element.text(content),
+ ])
+ }
+ |> element_to_raw_djotstring
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn preprocess_task_lists(djot: String) -> String { | |
| djot | |
| |> string.split("\n") | |
| |> list.map(fn(line) { | |
| let trimmed = string.trim(line) | |
| case string.starts_with(trimmed, "- [ ] ") { | |
| True -> { | |
| let content = string.drop_start(trimmed, 6) | |
| { | |
| html.div([attribute.class("flex items-center mb-2")], [ | |
| html.input([ | |
| attribute.type_("checkbox"), | |
| attribute.disabled(True), | |
| attribute.class("mr-2 accent-primary"), | |
| ]), | |
| element.text(content), | |
| ]) | |
| } | |
| |> element_to_raw_djotstring | |
| } | |
| False -> | |
| case | |
| string.starts_with(trimmed, "- [x] ") | |
| || string.starts_with(trimmed, "- [X] ") | |
| { | |
| True -> { | |
| let content = string.drop_start(trimmed, 6) | |
| "```=html\n<div class=\"flex items-center mb-2\"><input type=\"checkbox\" checked disabled class=\"mr-2 accent-primary\">" | |
| <> content | |
| <> "</div>\n```" | |
| } | |
| False -> line | |
| } | |
| } | |
| }) | |
| |> string.join("\n") | |
| } | |
| fn preprocess_task_lists(djot: String) -> String { | |
| djot | |
| |> string.split("\n") | |
| |> list.map(fn(line) { | |
| let trimmed = string.trim(line) | |
| case string.starts_with(trimmed, "- [ ] ") { | |
| True -> { | |
| let content = string.drop_start(trimmed, 6) | |
| { | |
| html.div([attribute.class("flex items-center mb-2")], [ | |
| html.input([ | |
| attribute.type_("checkbox"), | |
| attribute.disabled(True), | |
| attribute.class("mr-2 accent-primary"), | |
| ]), | |
| element.text(content), | |
| ]) | |
| } | |
| |> element_to_raw_djotstring | |
| } | |
| False -> | |
| case | |
| string.starts_with(trimmed, "- [x] ") | |
| || string.starts_with(trimmed, "- [X] ") | |
| { | |
| True -> { | |
| let content = string.drop_start(trimmed, 6) | |
| { | |
| html.div([attribute.class("flex items-center mb-2")], [ | |
| html.input([ | |
| attribute.type_("checkbox"), | |
| attribute.checked(True), | |
| attribute.disabled(True), | |
| attribute.class("mr-2 accent-primary"), | |
| ]), | |
| element.text(content), | |
| ]) | |
| } | |
| |> element_to_raw_djotstring | |
| } | |
| False -> line | |
| } | |
| } | |
| }) | |
| |> string.join("\n") | |
| } |
🤖 Prompt for AI Agents
In
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/djotparse.gleam
between lines 719 and 755, the unchecked task list items generate HTML using
Lustre element builders, but the checked items generate HTML as raw strings. To
fix this inconsistency, refactor the checked items to use the same Lustre
element building approach as the unchecked items by constructing the HTML
elements with html.div and html.input functions, setting the checked attribute
to True, and then converting the element to a raw djot string using
element_to_raw_djotstring.
| fn convert_table_to_raw(lines: List(String)) -> String { | ||
| case lines { | ||
| [header, separator, ..rows] -> { | ||
| case string.contains(separator, "|") && string.contains(separator, "-") { | ||
| True -> { | ||
| let header_cells = | ||
| header | ||
| |> string.split("|") | ||
| |> list.map(string.trim) | ||
| |> list.filter(fn(cell) { cell != "" }) | ||
|
|
||
| let data_rows = | ||
| rows | ||
| |> list.map(fn(row) { | ||
| row | ||
| |> string.split("|") | ||
| |> list.map(string.trim) | ||
| |> list.filter(fn(cell) { cell != "" }) | ||
| }) | ||
| |> list.filter(fn(row) { list.length(row) > 0 }) | ||
|
|
||
| let header_elements = { | ||
| list.map(header_cells, fn(cell) { | ||
| html.th([attribute.class("px-4 py-2 text-left font-bold")], [ | ||
| html.text(cell), | ||
| ]) | ||
| }) | ||
| } | ||
| let row_elements = { | ||
| list.map(data_rows, fn(row) { | ||
| html.tr([], { | ||
| list.map(row, fn(cell) { | ||
| html.td( | ||
| [ | ||
| attribute.class( | ||
| "px-4 py-2 border-t border-neutral-content", | ||
| ), | ||
| ], | ||
| [html.text(cell)], | ||
| ) | ||
| }) | ||
| }) | ||
| }) | ||
| } | ||
|
|
||
| html.table( | ||
| [ | ||
| attribute.class( | ||
| "table table-zebra w-full my-4 border border-neutral-content", | ||
| ), | ||
| ], | ||
| [ | ||
| html.thead([attribute.class("bg-neutral text-neutral-content")], [ | ||
| html.tr([], header_elements), | ||
| html.tbody([], row_elements), | ||
| ]), | ||
| ], | ||
| ) | ||
| |> element_to_raw_djotstring | ||
| } | ||
| False -> string.join(lines, "\n") | ||
| } | ||
| } | ||
| _ -> string.join(lines, "\n") | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Validate table structure more robustly
The convert_table_to_raw function assumes a specific table structure (header, separator, rows) but doesn't validate that the separator line is correctly formatted. Consider adding validation to ensure the separator contains the expected pattern of dashes and pipes.
fn convert_table_to_raw(lines: List(String)) -> String {
case lines {
[header, separator, ..rows] -> {
- case string.contains(separator, "|") && string.contains(separator, "-") {
+ // Validate separator line more strictly
+ let is_valid_separator =
+ string.contains(separator, "|") &&
+ string.contains(separator, "-") &&
+ // Ensure it's mostly dashes and pipes, not regular content
+ string.length(string.replace(string.replace(separator, "-", ""), "|", "") |> string.trim) < 5
+ case is_valid_separator {
True -> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn convert_table_to_raw(lines: List(String)) -> String { | |
| case lines { | |
| [header, separator, ..rows] -> { | |
| case string.contains(separator, "|") && string.contains(separator, "-") { | |
| True -> { | |
| let header_cells = | |
| header | |
| |> string.split("|") | |
| |> list.map(string.trim) | |
| |> list.filter(fn(cell) { cell != "" }) | |
| let data_rows = | |
| rows | |
| |> list.map(fn(row) { | |
| row | |
| |> string.split("|") | |
| |> list.map(string.trim) | |
| |> list.filter(fn(cell) { cell != "" }) | |
| }) | |
| |> list.filter(fn(row) { list.length(row) > 0 }) | |
| let header_elements = { | |
| list.map(header_cells, fn(cell) { | |
| html.th([attribute.class("px-4 py-2 text-left font-bold")], [ | |
| html.text(cell), | |
| ]) | |
| }) | |
| } | |
| let row_elements = { | |
| list.map(data_rows, fn(row) { | |
| html.tr([], { | |
| list.map(row, fn(cell) { | |
| html.td( | |
| [ | |
| attribute.class( | |
| "px-4 py-2 border-t border-neutral-content", | |
| ), | |
| ], | |
| [html.text(cell)], | |
| ) | |
| }) | |
| }) | |
| }) | |
| } | |
| html.table( | |
| [ | |
| attribute.class( | |
| "table table-zebra w-full my-4 border border-neutral-content", | |
| ), | |
| ], | |
| [ | |
| html.thead([attribute.class("bg-neutral text-neutral-content")], [ | |
| html.tr([], header_elements), | |
| html.tbody([], row_elements), | |
| ]), | |
| ], | |
| ) | |
| |> element_to_raw_djotstring | |
| } | |
| False -> string.join(lines, "\n") | |
| } | |
| } | |
| _ -> string.join(lines, "\n") | |
| } | |
| } | |
| fn convert_table_to_raw(lines: List(String)) -> String { | |
| case lines { | |
| [header, separator, ..rows] -> { | |
| // Validate separator line more strictly | |
| let is_valid_separator = | |
| string.contains(separator, "|") && | |
| string.contains(separator, "-") && | |
| // Ensure it's mostly dashes and pipes, not regular content | |
| string.length( | |
| string.replace( | |
| string.replace(separator, "-", ""), | |
| "|", | |
| "" | |
| ) | |
| |> string.trim | |
| ) < 5 | |
| case is_valid_separator { | |
| True -> { | |
| let header_cells = | |
| header | |
| |> string.split("|") | |
| |> list.map(string.trim) | |
| |> list.filter(fn(cell) { cell != "" }) | |
| let data_rows = | |
| rows | |
| |> list.map(fn(row) { | |
| row | |
| |> string.split("|") | |
| |> list.map(string.trim) | |
| |> list.filter(fn(cell) { cell != "" }) | |
| }) | |
| |> list.filter(fn(row) { list.length(row) > 0 }) | |
| let header_elements = { | |
| list.map(header_cells, fn(cell) { | |
| html.th([attribute.class("px-4 py-2 text-left font-bold")], [ | |
| html.text(cell), | |
| ]) | |
| }) | |
| } | |
| let row_elements = { | |
| list.map(data_rows, fn(row) { | |
| html.tr([], { | |
| list.map(row, fn(cell) { | |
| html.td( | |
| [ | |
| attribute.class( | |
| "px-4 py-2 border-t border-neutral-content", | |
| ), | |
| ], | |
| [html.text(cell)], | |
| ) | |
| }) | |
| }) | |
| }) | |
| } | |
| html.table( | |
| [ | |
| attribute.class( | |
| "table table-zebra w-full my-4 border border-neutral-content", | |
| ), | |
| ], | |
| [ | |
| html.thead([attribute.class("bg-neutral text-neutral-content")], [ | |
| html.tr([], header_elements), | |
| html.tbody([], row_elements), | |
| ]), | |
| ], | |
| ) | |
| |> element_to_raw_djotstring | |
| } | |
| False -> string.join(lines, "\n") | |
| } | |
| } | |
| _ -> string.join(lines, "\n") | |
| } | |
| } |
🤖 Prompt for AI Agents
In
cynthia_websites_mini_client/src/cynthia_websites_mini_client/pottery/djotparse.gleam
between lines 558 and 623, the convert_table_to_raw function currently checks
only if the separator line contains "|" and "-" but does not validate the
separator's format properly. Improve the validation by adding a check to ensure
the separator line matches the expected pattern of pipes and dashes (e.g., a
regex or a function that verifies each segment between pipes consists of
dashes). This will ensure the separator line is correctly formatted before
processing the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem for later...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@strawmelonjuice Understood! It's perfectly reasonable to focus on getting the core Djot implementation working first. The table validation improvement can definitely be addressed later.
Would you like me to create an issue to track this table validation enhancement as a follow-up item? I can provide a clear description of the improvement and link it back to this PR and comment for context.
🧠 Learnings used
Learnt from: strawmelonjuice
PR: CynthiaWebsiteEngine/Mini#33
File: cynthia_websites_mini_server/src/cynthia_websites_mini_server/config.gleam:896-899
Timestamp: 2025-07-10T13:40:49.876Z
Learning: In CynthiaWebsiteEngine Mini, the user prefers to avoid requiring external dependencies like Pandoc during initial setup to maintain a smooth out-of-the-box experience, even if it means accepting some compatibility risks with Markdown/Djot parsing differences.
Implements #28
Summary by CodeRabbit