-
Notifications
You must be signed in to change notification settings - Fork 0
V2 #1
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
base: main
Are you sure you want to change the base?
Conversation
…nvironment's built-in window global). Removed the unused node parameter from the smartDel rule's replacement function (this fixed the no-unused-vars error). In html-export-helper.js Removed the prior /* global window, DOMPurify */ and replaced it with /* global DOMPurify */ to declare DOMPurify as a known global without re-declaring window
…c into separate functions for improved readability and maintainability.
html-export-helper.test.js — tests sanitization, dark styles, and the DOMPurify branch. converter.extra.test.js and converter.more.test.js — tests for footnotes, tables, smartDel heuristic, ordered lists, custom list newline handling, empty-table behavior, and a roundtrip markdown → HTML → markdown. I converted the src modules to CommonJS-only modules to avoid unreachable browser-only UMD branches being counted as uncovered in Node/Jest
…ests for converter functionality
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ 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".
| // @ts-check | ||
| import { defineConfig, devices } from '@playwright/test'; | ||
|
|
||
| /** | ||
| * Playwright configuration focused on a single smoke test that assures the | ||
| * static `index.html` page renders without console errors. | ||
| */ | ||
| const captureArtifacts = process.env.PLAYWRIGHT_CAPTURE === '1'; | ||
|
|
||
| module.exports = defineConfig({ | ||
| testDir: './', | ||
| fullyParallel: true, |
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.
Fix Playwright config using ESM syntax in CommonJS module
The Playwright config is written with import … from while the project still defaults to CommonJS (package.json has no "type": "module"). Node will parse playwright.config.js as CommonJS and throw SyntaxError: Cannot use import statement outside a module before any e2e test runs, so npm run test:e2e:index and the pre‑push hook will always fail. Either convert the file (and the specs) to CommonJS require syntax or mark the file/package as ESM (e.g. rename to .mjs or set "type": "module").
Useful? React with 👍 / 👎.
Summary of ChangesHello @cs-util, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the project's development infrastructure by implementing a comprehensive automation and quality framework. The primary goal is to facilitate autonomous AI agent contributions through structured workflows and rigorous quality gates, while also enhancing the overall development experience for human contributors. It introduces new CI/CD processes, standardized issue/PR templates, and integrates advanced code quality tools to ensure a high standard of code health and maintainability. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces an impressive and comprehensive framework for automation and quality control, including CI/CD workflows for AI agents, extensive testing configurations, and detailed documentation. The setup is robust and establishes strong quality gates for the project. The refactoring of converter.js and html-export-helper.js improves code structure.
My main feedback concerns the duplication of logic. The refactored helper files in the root directory have been duplicated as CommonJS modules in the src/ directory, likely for testing purposes. This creates a significant maintenance issue. I've recommended consolidating to a single source of truth for these files. Additionally, I've suggested a minor tweak to the ESLint configuration to make the quality gates even stricter.
Overall, this is a fantastic contribution that significantly matures the project's development process.
| // src/converter.js — Lightweight HTML ⇄ Markdown converter (refactored) | ||
| // Works in both browser and Node.js environments. | ||
| // Dependencies: showdown, turndown, turndown-plugin-gfm | ||
|
|
||
| const Showdown = require('showdown'); | ||
| const TurndownService = require('turndown'); | ||
| const gfmPlugin = require('turndown-plugin-gfm').gfm; | ||
|
|
||
| function factory(Showdown, TurndownService, gfmPlugin) { | ||
| if (!Showdown || !TurndownService) { | ||
| throw new Error('Showdown and Turndown must be loaded before converter.js'); | ||
| } | ||
|
|
||
| const showdownConverter = new Showdown.Converter({ | ||
| tables: true, | ||
| strikethrough: true, | ||
| tasklists: true, | ||
| ghCodeBlocks: true, | ||
| parseImgDimensions: true, | ||
| ghCompatibleHeaderId: true, | ||
| requireSpaceBeforeHeadingText: true, | ||
| tablesHeaderId: true, | ||
| simpleLineBreaks: false, | ||
| footnotes: true, | ||
| ghMentions: false, | ||
| keepReferences: true, | ||
| }); | ||
| showdownConverter.setFlavor('github'); | ||
|
|
||
| const turndownService = new TurndownService({ | ||
| headingStyle: 'atx', | ||
| bulletListMarker: '-', | ||
| codeBlockStyle: 'fenced', | ||
| emDelimiter: '*', | ||
| strongDelimiter: '**', | ||
| linkStyle: 'inlined', | ||
| }); | ||
|
|
||
| if (gfmPlugin) { | ||
| turndownService.use(gfmPlugin); | ||
| } | ||
|
|
||
| turndownService.addRule('smartDel', { | ||
| filter: ['del'], | ||
| replacement: function (content) { | ||
| const htmlPattern = /[<>&]|html|tag|element|attribute/i; | ||
| if (htmlPattern.test(content)) { | ||
| return '<del>' + content + '</del>'; | ||
| } | ||
| return '~~' + content + '~~'; | ||
| }, | ||
| }); | ||
|
|
||
| // Helper to build headerLine and alignmentRow | ||
| function buildHeaderAlignment(headers) { | ||
| if (!headers.length) return { headerLine: '', alignmentRow: '' }; | ||
| const headerLine = | ||
| '| ' + headers.map((th) => th.textContent.trim()).join(' | ') + ' |'; | ||
| let alignmentRow = '|'; | ||
| for (const header of headers) { | ||
| const align = | ||
| header.style.textAlign || getComputedStyle(header).textAlign; | ||
| let marker; | ||
| switch (align) { | ||
| case 'left': | ||
| marker = ' :--- '; | ||
| break; | ||
| case 'center': | ||
| marker = ' :----: '; | ||
| break; | ||
| case 'right': | ||
| marker = ' ----: '; | ||
| break; | ||
| default: | ||
| marker = ' --- '; | ||
| break; | ||
| } | ||
| alignmentRow += marker + '|'; | ||
| } | ||
| return { headerLine, alignmentRow }; | ||
| } | ||
|
|
||
| // Helper to build body lines | ||
| function buildBodyLines(rows, headers) { | ||
| const bodyLines = []; | ||
| rows.forEach((row) => { | ||
| const cells = Array.from(row.querySelectorAll('td')); | ||
| const rowLine = | ||
| '| ' + | ||
| cells | ||
| .map((cell, idx) => { | ||
| const content = cell.textContent.trim(); | ||
| const align = | ||
| cell.style.textAlign || getComputedStyle(cell).textAlign; | ||
| if (align === 'center') { | ||
| const totalWidth = headers[idx] | ||
| ? headers[idx].textContent.trim().length | ||
| : content.length; | ||
| const padding = Math.max(totalWidth - content.length, 0); | ||
| const leftPad = Math.floor(padding / 2); | ||
| const rightPad = padding - leftPad; | ||
| return ' '.repeat(leftPad) + content + ' '.repeat(rightPad); | ||
| } else if (align === 'right') { | ||
| const totalWidth = headers[idx] | ||
| ? headers[idx].textContent.trim().length | ||
| : content.length; | ||
| const padding = Math.max(totalWidth - content.length, 0); | ||
| return ' '.repeat(padding) + content; | ||
| } | ||
| return content + ' '.repeat(4); | ||
| }) | ||
| .join(' | ') + | ||
| ' |'; | ||
| bodyLines.push(rowLine); | ||
| }); | ||
| return bodyLines; | ||
| } | ||
|
|
||
| // Custom list item rule (kept small) | ||
| turndownService.addRule('customListItem', { | ||
| filter: 'li', | ||
| replacement: function (content, node, options) { | ||
| content = content.replace(/\\n/g, '\n'); | ||
| let isNestedListContent = content.startsWith(' '); | ||
| if (!isNestedListContent) content = content.trim(); | ||
| if (content.includes('\n') && !isNestedListContent) { | ||
| content = content.replace(/\n/gm, '\n '); | ||
| } | ||
|
|
||
| var prefix = ''; | ||
| var parent = node.parentNode; | ||
| if (parent.nodeName === 'OL') { | ||
| var start = parent.getAttribute('start'); | ||
| var index = Array.prototype.indexOf.call(parent.children, node); | ||
| prefix = (start ? Number(start) + index : index + 1) + '. '; | ||
| } else { | ||
| prefix = options.bulletListMarker + ' '; | ||
| } | ||
|
|
||
| let trailingNewline = ''; | ||
| if (node.nextSibling) { | ||
| trailingNewline = '\n'; | ||
| } else if (content.includes('\n')) { | ||
| trailingNewline = '\n'; | ||
| } | ||
|
|
||
| return prefix + content + trailingNewline; | ||
| }, | ||
| }); | ||
|
|
||
| turndownService.keep(['kbd']); | ||
|
|
||
| turndownService.addRule('tableCell', { | ||
| filter: ['th', 'td'], | ||
| replacement: function (content, node) { | ||
| const trimmedContent = content.trim(); | ||
| const align = node.style.textAlign || getComputedStyle(node).textAlign; | ||
| if (align === 'center') return ' ' + trimmedContent + ' '; | ||
| if (align === 'right') return ' ' + trimmedContent + ' '; | ||
| return ' ' + trimmedContent + ' '; | ||
| }, | ||
| }); | ||
|
|
||
| turndownService.addRule('tableWithAlignment', { | ||
| filter: 'table', | ||
| replacement: function (content, node) { | ||
| if (!content.trim()) return ''; | ||
| const headers = Array.from(node.querySelectorAll('thead th')); | ||
| let { headerLine, alignmentRow } = buildHeaderAlignment(headers); | ||
| let rows = Array.from(node.querySelectorAll('tbody tr')); | ||
| let bodyLines = buildBodyLines(rows, headers); | ||
|
|
||
| if (!headerLine && rows.length > 0) { | ||
| const firstRowCells = Array.from(rows[0].querySelectorAll('td')); | ||
| if (firstRowCells.length > 0) { | ||
| headerLine = | ||
| '| ' + | ||
| firstRowCells.map((cell) => cell.textContent.trim()).join(' | ') + | ||
| ' |'; | ||
| alignmentRow = | ||
| '| ' + firstRowCells.map(() => '---').join(' | ') + ' |'; | ||
| bodyLines.shift(); | ||
| } else { | ||
| return ''; | ||
| } | ||
| } else if (!headerLine && rows.length === 0) { | ||
| return ''; | ||
| } | ||
|
|
||
| return ( | ||
| headerLine + | ||
| '\n' + | ||
| alignmentRow + | ||
| (bodyLines.length > 0 ? '\n' + bodyLines.join('\n') : '') | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| turndownService.addRule('footnoteRef', { | ||
| filter: (node) => { | ||
| if (node.nodeName === 'A' && node.getAttribute('rel') === 'footnote') { | ||
| const href = node.getAttribute('href'); | ||
| if (href && href.startsWith('#fn-')) return true; | ||
| } | ||
| return false; | ||
| }, | ||
| replacement: (content, node) => { | ||
| const href = node.getAttribute('href'); | ||
| const footnoteId = href.substring(4); | ||
| return `[^${footnoteId}]`; | ||
| }, | ||
| }); | ||
|
|
||
| turndownService.addRule('footnoteDefinition', { | ||
| filter: (node) => | ||
| node.nodeName === 'LI' && node.id && node.id.startsWith('fn-'), | ||
| replacement: (content, node) => { | ||
| const id = node.id.substring(3); | ||
| const clonedNode = node.cloneNode(true); | ||
| const backlink = clonedNode.querySelector('a[rev="footnote"]'); | ||
| if (backlink) backlink.parentNode.removeChild(backlink); | ||
| let footnoteText = turndownService.turndown(clonedNode.innerHTML).trim(); | ||
| return `[^${id}]: ${footnoteText}\n`; | ||
| }, | ||
| }); | ||
|
|
||
| function markdownToHtml(markdown) { | ||
| return showdownConverter.makeHtml(markdown); | ||
| } | ||
|
|
||
| function htmlToMarkdown(html) { | ||
| let markdown = turndownService.turndown(html); | ||
| markdown = markdown.replace(/\\\[\^([\w\d-]+)\\\]/g, '[^$1]'); | ||
| markdown = markdown.replace(/\\\[\^([\w\d-]+)\\\]:/g, '[^$1]:'); | ||
| markdown = markdown.replace(/\\\[\^([^\]]+)\\\]/g, '[^$1]'); | ||
| return markdown; | ||
| } | ||
|
|
||
| return { markdownToHtml, htmlToMarkdown }; | ||
| } | ||
|
|
||
| module.exports = factory(Showdown, TurndownService, gfmPlugin); |
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.
This file duplicates the logic from converter.js located in the root directory. Having two files with the same logic creates a significant maintenance burden, as any changes must be manually synchronized between them. This can easily lead to inconsistencies and bugs.
To resolve this, I recommend establishing a single source of truth. Since the project aims for a 'no-build' setup for the browser, the UMD-wrapped file in the root (/converter.js) is necessary. Jest is capable of consuming UMD files, so the tests can be updated to import from the root file.
Please remove this file (src/converter.js) and update the corresponding tests in tests/ to import ../converter.js instead.
| // src/html-export-helper.js — HTML Export Utilities (refactored) | ||
| /* global DOMPurify */ | ||
| function factory() { | ||
| function sanitizeContent(content) { | ||
| if (typeof DOMPurify !== 'undefined' && DOMPurify.sanitize) { | ||
| return DOMPurify.sanitize(content, { | ||
| KEEP_CONTENT: true, | ||
| ALLOWED_TAGS: [ | ||
| 'p', | ||
| 'br', | ||
| 'strong', | ||
| 'em', | ||
| 'u', | ||
| 's', | ||
| 'h1', | ||
| 'h2', | ||
| 'h3', | ||
| 'h4', | ||
| 'h5', | ||
| 'h6', | ||
| 'ul', | ||
| 'ol', | ||
| 'li', | ||
| 'blockquote', | ||
| 'a', | ||
| 'img', | ||
| 'table', | ||
| 'thead', | ||
| 'tbody', | ||
| 'tr', | ||
| 'th', | ||
| 'td', | ||
| 'code', | ||
| 'pre', | ||
| 'span', | ||
| 'div', | ||
| 'sub', | ||
| 'sup', | ||
| ], | ||
| ALLOWED_ATTR: [ | ||
| 'href', | ||
| 'src', | ||
| 'alt', | ||
| 'title', | ||
| 'class', | ||
| 'style', | ||
| 'target', | ||
| 'rel', | ||
| ], | ||
| }); | ||
| } | ||
|
|
||
| return content | ||
| .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') | ||
| .replace(/<iframe\b[^<]*(?:(?!<\/iframe>)<[^<]*)*<\/iframe>/gi, '') | ||
| .replace(/javascript:/gi, '') | ||
| .replace(/on\w+="[^"]*"/gi, '') | ||
| .replace(/on\w+='[^']*'/gi, ''); | ||
| } | ||
|
|
||
| function buildStyles(isDark) { | ||
| return ` | ||
| body { | ||
| font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; | ||
| line-height: 1.6; | ||
| max-width: 800px; | ||
| margin: 0 auto; | ||
| padding: 20px; | ||
| color: ${isDark ? '#e0e0e0' : '#333'}; | ||
| background-color: ${isDark ? '#1e1e1e' : '#fff'}; | ||
| } | ||
| h1, h2, h3, h4, h5, h6 { | ||
| color: ${isDark ? '#fff' : '#000'}; | ||
| margin: 1.5em 0 0.5em 0; | ||
| } | ||
| code, pre { font-family: "Monaco", "Consolas", monospace; background-color: ${isDark ? '#374151' : '#f5f5f5'}; } | ||
| code { padding: 0.2em 0.4em; border-radius: 3px; } | ||
| pre { padding: 1em; border-radius: 5px; overflow-x: auto; } | ||
| pre code { background: transparent; padding: 0; } | ||
| blockquote { border-left: 4px solid ${isDark ? '#4b5563' : '#ddd'}; margin: 1em 0; padding-left: 1em; color: ${isDark ? '#a0a0a0' : '#666'}; } | ||
| table { border-collapse: collapse; width: 100%; margin: 1em 0; } | ||
| th, td { border: 1px solid ${isDark ? '#4b5563' : '#ddd'}; padding: 8px 12px; text-align: left; } | ||
| th { background-color: ${isDark ? '#374151' : '#f9f9f9'}; font-weight: bold; } | ||
| img { max-width: 100%; height: auto; } | ||
| a { color: ${isDark ? '#60a5fa' : '#0066cc'}; } | ||
| .ql-align-center { text-align: center; } | ||
| .ql-align-right { text-align: right; } | ||
| .ql-align-justify { text-align: justify; } | ||
| @media print { body { color: #000; background: #fff; } } | ||
| `; | ||
| } | ||
|
|
||
| function createCompleteHtmlDocument(content, title, isDark = false) { | ||
| const sanitizedContent = sanitizeContent(content); | ||
| const styles = buildStyles(isDark); | ||
| return `<!DOCTYPE html> | ||
| <html lang="en"> | ||
| <head> | ||
| <meta charset="UTF-8"> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <title>${title}</title> | ||
| <style>${styles}</style> | ||
| </head> | ||
| <body> | ||
| ${sanitizedContent} | ||
| </body> | ||
| </html>`; | ||
| } | ||
|
|
||
| return { createCompleteHtmlDocument }; | ||
| } | ||
|
|
||
| module.exports = factory(); |
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.
Similar to src/converter.js, this file duplicates the logic from html-export-helper.js in the root directory. This introduces a maintenance risk, as the two files can diverge over time.
To ensure there is a single source of truth, I recommend removing this file and updating the tests to use the UMD module from the root directory (/html-export-helper.js). This will simplify maintenance and prevent potential bugs from unsynchronized changes.
| complexity: ['warn', 10], | ||
| 'max-depth': ['warn', 4], |
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.
The complexity rules (complexity and max-depth) are currently set to 'warn'. To better enforce code quality and maintainability, it's recommended to set these to 'error'. This will make the linting process fail for overly complex code, turning this quality check into a strict gate rather than a suggestion. This aligns with the project's goal of enforcing high quality standards.
| complexity: ['warn', 10], | |
| 'max-depth': ['warn', 4], | |
| complexity: ['error', 10], | |
| 'max-depth': ['error', 4], |
This pull request introduces a comprehensive automation and quality framework for the project, focusing on enabling autonomous AI agents (such as Claude and Cursor) to contribute via GitHub issues and pull requests, while enforcing high quality standards. The changes include new CI workflows, pre-push hooks, and detailed issue templates for agent-driven development, bug reports, and feature requests.
The most important changes are:
Automation and Agent Integration:
Continuous Integration and Quality Gates:
Issue and PR Templates for Agent-Driven Development:
Tooling and Configuration: