Fix Windows/macOS builds, log formatting, markdown rendering, and user mentions #92
Conversation
- Windows build: Patch CMakeLists.txt for CMake 3.5+ compatibility - Log formatting: Add _ansi_to_html_for_pre for proper HTML in <pre> tags - Markdown: Convert markdown to HTML for log messages - Mentions: Use full Matrix IDs with HTML links for proper highlighting Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
- Removed old summary files (AUDIT_SUMMARY, BUILD_FIX_SUMMARY, etc.) - Removed overly detailed design docs (TUI_TURBO_VISION_DESIGN, etc.) - Removed redundant CONCURRENT_ARCHITECTURE and BRANDING docs - Added tests for new _ansi_to_html_for_pre function - Added tests for markdown mention conversion - Added test for _get_display_name returning full Matrix IDs Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
- Simplified _ansi_to_html_for_pre to strip ANSI codes instead of converting to HTML - Updated tests to match simplified implementation - Fixed regex pattern in build.yml for better version matching - Updated CHANGELOG to reflect simplified ANSI handling Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
- Enhanced Matrix mention regex to support special chars (+, _, =) - Added validation for CMake patch to ensure it succeeds - Added test for mentions with special characters - Improved error handling in build workflow Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
|
@copilot Mac can't compile because: Getting requirements to build wheel: started × Getting requirements to build wheel did not run successfully. note: This error originates from a subprocess, and is likely not a problem with pip. × Getting requirements to build wheel did not run successfully. note: This error originates from a subprocess, and is likely not a problem with pip. |
Set CFLAGS and LDFLAGS environment variables to ensure pip uses the homebrew-installed libolm instead of trying to build from bundled source. Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
Fixed in commit 1cb3933. The issue was that pip was trying to build libolm from the bundled source in python-olm instead of using the homebrew-installed version. I added CFLAGS and LDFLAGS environment variables to point pip to the homebrew libolm, which prevents it from building from source. |
|
@copilot Unit test: ======================================================================
|
Updated test expectations to match the new behavior where _get_display_name returns full Matrix IDs (@user:server.com) instead of truncated usernames. Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
Fixed in commit f92f291. The test was expecting the old behavior where |
|
Run docker://rhysd/actionlint:latest During build unit test |
Use command grouping with braces to redirect multiple echo statements to GITHUB_ENV more efficiently, addressing shellcheck style warning. Co-authored-by: CJFWeatherhead <6923098+CJFWeatherhead@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR performs a documentation cleanup, fixing user mention rendering, improving log formatting for Matrix clients, and resolving build workflow compatibility issues.
- Removes 11 outdated/redundant documentation files that accumulated during development (over 4000 lines of documentation bloat)
- Fixes user mention rendering by returning full Matrix IDs and converting them to clickable HTML links
- Fixes log output formatting in
!cd logcommand to properly display line breaks in Matrix clients - Patches Windows build workflow to support modern CMake versions and improves macOS build configuration
Reviewed Changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_commands.py | Updates tests for new mention rendering behavior and adds tests for ANSI-to-HTML conversion for <pre> tags |
| docs/TUI_TURBO_VISION_DESIGN.md | Removes outdated TUI design documentation (311 lines) |
| docs/TUI_MIGRATION_GUIDE.md | Removes obsolete TUI migration guide (278 lines) |
| docs/BUILD_WORKFLOW.md | Removes redundant build workflow documentation (351 lines) |
| chatrixcd/commands.py | Fixes user mentions to return full Matrix IDs, adds _ansi_to_html_for_pre() for proper log formatting, and adds HTML conversion to log messages |
| VERIFICATION_FIX_SUMMARY.md | Removes development summary file (323 lines) |
| TURBO_VISION_SUMMARY.md | Removes TUI implementation summary (328 lines) |
| TESTING_REVIEW.md | Removes testing review document (196 lines) |
| TESTING_INSTRUCTIONS.md | Removes build testing instructions (142 lines) |
| IMPLEMENTATION_SUMMARY.md | Removes implementation summary document (255 lines) |
| IMPLEMENTATION_NOTES.md | Removes concurrent architecture notes (293 lines) |
| FEATURE_SUMMARY.md | Removes feature implementation summary (248 lines) |
| ENCRYPTION_FIX_SUMMARY.md | Removes encryption fix documentation (435 lines) |
| DOCUMENTATION_UPDATE_SUMMARY.md | Removes documentation update summary (152 lines) |
| CONCURRENT_ARCHITECTURE.md | Removes concurrent architecture documentation (252 lines) |
| CHANGELOG.md | Documents all fixes and cleanup changes |
| BUILD_FIX_SUMMARY.md | Removes build fix summary (146 lines) |
| BRANDING.md | Removes branding guide (209 lines) |
| AUDIT_SUMMARY.md | Removes audit summary document (328 lines) |
| .github/workflows/build.yml | Patches Windows libolm CMake requirements and adds macOS compiler flags for homebrew libolm |
| text = re.sub( | ||
| r'(@[a-zA-Z0-9._=+-]+:[a-zA-Z0-9.-]+\.[a-zA-Z]{2,})', | ||
| r'<a href="https://matrix.to/#/\1">\1</a>', | ||
| text | ||
| ) |
There was a problem hiding this comment.
The regex pattern for Matrix user mentions doesn't match uppercase letters in the localpart, but the comment on line 106 states 'lowercase letters, digits...'. However, Matrix user IDs can contain uppercase letters in the localpart. The pattern should include A-Z in the first character class or the comment should be corrected to match the implementation.
|
|
||
| # Patch CMakeLists.txt to require CMake 3.5+ (for compatibility with modern CMake) | ||
| $originalContent = Get-Content CMakeLists.txt -Raw | ||
| $patchedContent = $originalContent -replace 'cmake_minimum_required\(VERSION [0-9]+\.[0-9]+(?:\.[0-9]+)?\)', 'cmake_minimum_required(VERSION 3.5)' |
There was a problem hiding this comment.
[nitpick] The regex pattern uses PowerShell string escaping but the parentheses in cmake_minimum_required are not escaped. While this works, it's more correct to escape them as \( and \) for clarity, or use a verbatim string pattern. Consider: 'cmake_minimum_required\\(VERSION [0-9]+\\.[0-9]+(?:\\.[0-9]+)?\\)' for better maintainability.
| $patchedContent = $originalContent -replace 'cmake_minimum_required\(VERSION [0-9]+\.[0-9]+(?:\.[0-9]+)?\)', 'cmake_minimum_required(VERSION 3.5)' | |
| $patchedContent = $originalContent -replace 'cmake_minimum_required\\(VERSION [0-9]+\\.[0-9]+(?:\\.[0-9]+)?\\)', 'cmake_minimum_required(VERSION 3.5)' |
Comprehensive fix for multiple issues in ChatrixCD
Issues Addressed
Changes Made
1. Build Fixes
Windows Build - CMake Compatibility
macOS Build
2. Log Formatting
_ansi_to_html_for_pre()method that strips ANSI codes and preserves newlines_format_task_logs_html()to use the new method<pre>tags (no<br>needed)3. Markdown & Mentions
markdown_to_html()to convert Matrix mentions (@user:server.com) to HTML links_get_display_name()to return full Matrix IDs4. Test Coverage
_ansi_to_html_for_pre()verifying ANSI stripping and newline preservation_get_display_name()returning full Matrix IDs5. Documentation Cleanup
6. Code Quality Improvements
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.