Feature/issue 56/create new output mode#73
Conversation
Add support for output mode configuration to allow users to choose between list and table formats. This adds the necessary input parameter and validation logic following the project's existing patterns.
Implement formatEventsAsTable function to generate markdown and HTML table formats. The formatter supports both output styles and properly escapes special characters to prevent table formatting issues.
Extend OUTPUT_MODE configuration to accept 'svg' mode in addition to 'list' and 'table'. This enables generating SVG image files as an alternative output format.
Implement formatEventsAsSVG function to generate a clean, GitHub-styled SVG image. Features include: - Card-based design with alternating row backgrounds - Selectable text rendering - Proper text wrapping for long descriptions - Date formatting and event numbering - Responsive layout with automatic height calculation - XML escaping for safe rendering
Modify fetchAndFilterEvents to return an object containing both the formatted string (for backward compatibility) and raw event data (for new output modes like SVG). This allows different formatters to access the original event data.
Implement writeSvgFile function to generate and commit SVG files. Add formatActivity helper to route between list and table modes. The SVG writer follows the same patterns as updateReadme with: - Change detection to avoid unnecessary commits - Dry run support - Proper Git commit with github-actions[bot] author
Update main entry point to route between README updates and SVG generation based on OUTPUT_MODE. Add informative logging to show processing status and mode selection. This completes the integration of all three output modes: list, table, and svg.
Rebuild dist/index.js with all new features including: - Table output mode support - SVG generation functionality - Enhanced output mode routing - Updated event formatting utilities
Add test workflows and standalone test scripts for validating all three output modes: - GitHub Actions workflows for local testing with act - Standalone Node.js test script with mock data - Comprehensive testing documentation in TESTING.md - Test scripts support list, table, and SVG modes
Add test-output.svg to gitignore to prevent committing temporary test files.
Move all test-related files into a dedicated test/ directory following standard project structure conventions: - test-*.js → test/*.js (remove redundant 'test-' prefix) - TESTING.md → test/README.md - Update paths in documentation and scripts - Update .gitignore to reflect new test output location This improves organization and follows common practices from Python and other ecosystems.
Add comprehensive documentation for the new OUTPUT_MODE feature: - Add to features list - Document all three modes (list, table, svg) in Inputs table - Create new "Output Modes" section with: - Detailed descriptions for each mode - Usage examples with workflow snippets - Visual examples of output formats - SVG mode benefits and use cases - Instructions for embedding SVG in README Addresses issue TheDanniCraft#56 documentation requirements.
|
Really like the implementation! I don't have time to properly review it today, it's almost 1 AM for me and I really need to get some sleep. For now, I'll request a review from Copilot to get some initial feedback and keep things moving. Please make sure to double-check any suggested changes, since Copilot's reviews are AI-generated and might not always make sense. One thing I noticed: Maybe it's just on my end, but the text in the SVG doesn't seem to be correctly aligned (it's slightly lower). |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new OUTPUT_MODE configuration that enables users to choose between three different output formats for their activity log: list (default), table, and SVG image generation. The feature addresses the need for reduced commit noise by allowing SVG files to be generated separately instead of updating READMEs directly.
Key Changes:
- Added
OUTPUT_MODEinput parameter supporting list, table, and SVG formats - Implemented SVG generator with GitHub-styled design and dynamic height calculation
- Enhanced
fetchAndFilterEventsto return both formatted strings and raw event data - Full backward compatibility maintained with list mode as default
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| action.yml | Adds OUTPUT_MODE input definition with list/table/svg options |
| src/config.js | Adds validation for OUTPUT_MODE parameter |
| src/utils/eventDescriptions.js | Implements table and SVG formatters with text wrapping and XML escaping |
| src/utils/github.js | Modifies return structure to include both formatted strings and raw events |
| src/utils/file.js | Adds SVG file writer and table mode support with Git operations |
| src/index.js | Implements output mode routing logic |
| test/*.js | Adds standalone, local, and quick test scripts |
| test/README.md | Provides comprehensive testing guide |
| .github/workflows/test-*.yml | Adds workflow tests for each output mode |
| README.md | Documents the new OUTPUT_MODE feature with examples |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
TheDanniCraft
left a comment
TheDanniCraft
left a comment
There was a problem hiding this comment.
Looks good overall! 👌
From a code-only review, everything seems fine (I’ll test it once I’m home).
One small concern: having multiple workflows:
.github/workflows/test-list-mode.yml.github/workflows/test-svg-mode.yml.github/workflows/test-table-mode.yml
might be a bit confusing for users. It’s not immediately clear which one they should use or why there are several.
Since update-activity.yml is currently the main "reference" workflow, maybe we could clarify that in the README or turn the others into example workflows and move them into an examples/ folder to avoid confusion.
Other than that, I really like that you added tests, nice work!
Maybe also take a look at the Copilot review comments on your tests and see if any of them are worth addressing.
- Fix test/quick.js: use '../src/utils/eventDescriptions' - Fix test/local.js: use '../src/index.js' - Addresses Copilot AI feedback on PR TheDanniCraft#73
- Add truncation logic to wrapText() for words exceeding maxWidth - Truncate long words to maxWidth-3 and append '...' - Adjust SVG text alignment using rowCenterY with dominant-baseline="middle" - Ensures proper vertical centering and prevents layout breaks - Addresses Copilot AI and maintainer feedback on PR TheDanniCraft#73
…tions - Cache normalized strings before comparison in writeSvgFile() - Reduces duplicate regex operations from 2 to 1 per comparison - Improves performance when checking SVG content changes - Addresses Copilot AI performance feedback on PR TheDanniCraft#73
- Move .github/workflows/test-*-mode.yml to .github/workflows/examples/ - Add "Example Workflows" section in README.md referencing examples - Update test/README.md to clarify workflows are examples - Prevents confusion between real workflows and testing examples - Addresses maintainer feedback on PR TheDanniCraft#73
- Rebuild dist/index.js with all PR TheDanniCraft#73 review fixes - Includes: test path fixes, SVG text wrapping, performance optimization - Generated with bun run build
- Fix test/quick.js: use '../src/utils/eventDescriptions' - Fix test/local.js: use '../src/index.js' Addresses Copilot AI feedback on PR TheDanniCraft#73
- Add truncation logic to wrapText() for words exceeding maxWidth - Truncate long words to maxWidth-3 and append '...' - Adjust SVG text alignment using rowCenterY with dominant-baseline="middle" - Ensures proper vertical centering and prevents layout breaks Addresses Copilot AI and maintainer feedback on PR TheDanniCraft#73
…tions - Cache normalized strings before comparison in writeSvgFile() - Reduces duplicate regex operations from 2 to 1 per comparison - Improves performance when checking SVG content changes Addresses Copilot AI performance feedback on PR TheDanniCraft#73
- Move .github/workflows/test-*-mode.yml to .github/workflows/examples/ - Add "Example Workflows" section in README.md referencing examples - Update test/README.md to clarify workflows are examples - Prevents confusion between real workflows and testing examples - Add issues:write permission to commitlint workflow - Make github-token explicit in post-comment action Addresses maintainer feedback and CI permission issues on PR TheDanniCraft#73
- Rebuild dist/index.js with all PR TheDanniCraft#73 review fixes - Includes: test path fixes, SVG text wrapping, performance optimization - Includes: workflow reorganization and CI permission fixes Generated with bun run build
pizofreude
force-pushed
the
feature/issue-56/create-new-output-mode
branch
from
16df66a to
faf6fb6
Compare
|
All fixes completed. |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 15 out of 17 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
TheDanniCraft
left a comment
TheDanniCraft
left a comment
There was a problem hiding this comment.
This looks good overall.
A few small things to wrap up:
- Could you please check the Copilot review comments and see if there is anything worth addressing?
- Please remove the docstrings to keep things consistent.
- For the tests, would it make sense to turn them into proper Jest tests? That might make them easier to maintain and integrate.
- The text alignment looks a bit off. Could you take another look at that? (see screenshot)
There was a problem hiding this comment.
Updating the workflow or commitlint is out of scope for this PR. We are currently experimenting with the minimal scoping variant.
There was a problem hiding this comment.
I had to do that during troubleshooting before since the PR checks didn't pass but shows zero error. I found out for perf I need to use :zap: instead of ⚡. So I've now reverted the commitlint changes. For the SVG alignment problem, I decided to simply remove the date which isn't shown anyway in Markdown output. So, it fixes the alignment problem.
- Replace two-line format (date + description) with elegant - single-line format showing only event description.
Add quick length comparison before expensive regex operations to avoid unnecessary string normalization on large SVG files. Addresses Copilot review comment about performance optimization.
Replace standalone test scripts with industry-standard Jest testing framework. Changes: - Add Jest as dev dependency - Create jest.config.js with coverage settings - Add test scripts to package.json - Implement 26 comprehensive unit tests: - 21 tests for formatters (table & SVG) - 5 tests for config validation - Test coverage for edge cases and error conditions Benefits: - Automated testing with proper assertions - Coverage reports (aim for >80%) - CI/CD integration ready - Industry standard approach All tests passing: 26 pass, 0 fail, 52 expect() calls
Reorganize test structure following Jest conventions:
- Move tests to __tests__/unit/ subdirectory
- Create comprehensive testing documentation
- Remove redundant test/ directory and standalone scripts
Structure:
__tests__/
├── README.md (testing guide)
└── unit/
├── config.test.js
└── formatters.test.js
Removed:
- test/local.js (replaced by Jest)
- test/quick.js (replaced by Jest)
- test/standalone.js (replaced by Jest)
- test/README.md (moved to __tests__)
Benefits:
- Single source of truth for tests
- Industry standard Jest convention
- Clear organization (unit/ subdirectory)
- No confusion from duplicate directories
Add Jest coverage directory to gitignore. Remove reference to deleted test/output.svg file.
Rebuild distribution bundle with: - New single-line SVG format - Performance optimizations - All code review fixes applied Distribution size: ~2.2MB (minified) All 26 Jest tests passing
|
Huh, just like before, the same problem with gitmoji + conventional commit. @TheDanniCraft please fix this, it's really frustrating to troubleshoot something about gitmoji in the commit message instead of the bug in the code.
|
|
@pizofreude Both 👥 and ⚡ are part of the official Gitmoji standard, but |
That's what happen when using the emoji not supported by the 
|
|
@TheDanniCraft @pizofreude 👥 works for me. ⚡not. But @pizofreude, the emojis are invalid. I also get a � (Replacement Character) instead of an actual emoji. When I rebase the commit to use the actual real emojis 👥 works. |
I see some inconsistency on how the This is the least safe net to avoid headaches for future contributors due to ambiguous error code this bug returns. Literally the error code: Found 0 problems, 0 warnings -> Error: Process completed with exit code 1. Cited from StackOverflow regarding the error code: "indicates that a process in a workflow, script, or container has failed, as any non-zero exit code signifies failure." |
TheDanniCraft
left a comment
TheDanniCraft
left a comment
There was a problem hiding this comment.
@pizofreude Almost there, thanks for your contribution. The implementation looks good overall, just a few small details to address before merging.
- Building the project creates new files. Please make sure you build your project before committing.
- The tests are failing. I left comments on the related files.
| it('should accept table mode', () => { | ||
| mockCore.getInput.mockImplementation((name) => { | ||
| const inputs = { | ||
| 'GITHUB_USERNAME': 'testuser', | ||
| 'GITHUB_TOKEN': 'token123', | ||
| 'OUTPUT_MODE': 'table', | ||
| 'OUTPUT_STYLE': 'MARKDOWN', | ||
| 'README_PATH': 'README.md', | ||
| 'EVENT_LIMIT': '5', | ||
| 'IGNORE_EVENTS': '[]', | ||
| 'HIDE_DETAILS_ON_PRIVATE_REPOS': 'false', | ||
| 'COMMIT_MESSAGE': 'test', | ||
| 'EVENT_EMOJI_MAP': '', | ||
| 'DRY_RUN': 'false' | ||
| }; | ||
| return inputs[name] || ''; | ||
| }); | ||
|
|
||
| delete require.cache[require.resolve('../../src/config')]; | ||
| const config = require('../../src/config'); | ||
| expect(config.outputMode).toBe('table'); | ||
| }); |
There was a problem hiding this comment.
This test is currently failing with the following error:
● Config Module › processOutputMode › should accept table mode
expect(received).toBe(expected) // Object.is equality
Expected: "table"
Received: "list"
56 | delete require.cache[require.resolve('../../src/config')];
57 | const config = require('../../src/config');
> 58 | expect(config.outputMode).toBe('table');
| ^
59 | });
60 |
61 | it('should accept svg mode', () => {
at Object.toBe (__tests__/unit/config.test.js:58:39)
| expect(config.outputMode).toBe('table'); | ||
| }); | ||
|
|
||
| it('should accept svg mode', () => { |
There was a problem hiding this comment.
This test is currently failing with the following error:
● Config Module › processOutputMode › should accept svg mode
expect(received).toBe(expected) // Object.is equality
Expected: "svg"
Received: "list"
79 | delete require.cache[require.resolve('../../src/config')];
80 | const config = require('../../src/config');
> 81 | expect(config.outputMode).toBe('svg');
| ^
82 | });
83 | });
84 |
at Object.toBe (__tests__/unit/config.test.js:81:39)
| expect(config.style).toBe('MARKDOWN'); | ||
| }); | ||
|
|
||
| it('should accept HTML style', () => { |
There was a problem hiding this comment.
This test is currently failing with the following error:
● Config Module › processOutputStyle › should accept HTML style
expect(received).toBe(expected) // Object.is equality
Expected: "HTML"
Received: "MARKDOWN"
127 | delete require.cache[require.resolve('../../src/config')];
128 | const config = require('../../src/config');
> 129 | expect(config.style).toBe('HTML');
| ^
130 | });
131 | });
132 | });
at Object.toBe (__tests__/unit/config.test.js:129:34)
There was a problem hiding this comment.
The test suite is failing with the following error:
FAIL __tests__/unit/formatters.test.js
● Test suite failed to run
Input required and not supplied: GITHUB_USERNAME
153 | // Load inputs from GitHub Actions
154 | module.exports = {
> 155 | username: core.getInput('GITHUB_USERNAME', { required: true }),
| ^
156 | token: core.getInput('GITHUB_TOKEN', { required: true }),
157 | eventLimit: processEventLimit(core.getInput('EVENT_LIMIT')),
158 | style: processStyle(core.getInput('OUTPUT_STYLE')),
at Object.getInput (node_modules/@actions/core/src/core.ts:130:11)
at Object.getInput (src/config.js:155:20)
at Object.require (src/utils/eventDescriptions.js:1:27)
at Object.require (__tests__/unit/formatters.test.js:1:52)
@pizofreude I'm sorry for that, I tried copying them from gitmoji.dev as well and got the same results: 👥 worked, ⚡ did not. Using the emoji keyboard or copying it from a text file didn’t change anything either. I'm not 100% happy with |
Like I mentioned before, using :shortcode: from gitmoji.dev convention solves this issue for the unsupported emojis. In the end, the :shortcode: will be automatically rendered as the required emoji. So, I really didn't see any downside in adding this as a note in the |
|
@pizofreude I’ve opened PR #83 that adds a note to the Is this how you’d like the notice to be worded? |
LGTM! |
🚀 Pull Request
📝 Description
This PR implements a new
OUTPUT_MODEconfiguration that allows users to choose between three different output formats for their activity log: list (default), table, and SVG image generation.The SVG mode addresses the core request in the issue by generating a separate image file that can be manually embedded in READMEs, avoiding commit noise for users who prefer automation repositories.
Key improvements:
Fixes #56
🧩 Type of Change
📋 Checklist
nektos/act)💬 Other Information
🎯 Features Implemented
1. List Mode (Default)
Original numbered list format - maintains 100% backward compatibility.
2. Table Mode
Generates a clean markdown/HTML table with columns:
Example output:
3. SVG Mode
Generates a beautiful, GitHub-styled SVG image (
activity-log.svg) with:Users can embed it in their README:
🧪 Testing
Comprehensive test suite added in
test/directory:test/standalone.js- Quick test with mock data (verified working)test/local.js- Full integration test with real GitHub APItest/README.md- Complete testing documentation.github/workflows/test-*.yml- Workflow tests for each modeTest results:
📦 Files Modified
Core Implementation (8 files):
action.yml- Added OUTPUT_MODE input definitionsrc/config.js- Added mode validation (list/table/svg)src/utils/eventDescriptions.js- Table & SVG formatters (~150 lines)src/utils/github.js- Return both formatted and raw eventssrc/utils/file.js- SVG writer + table supportsrc/index.js- Output mode routing logicdist/index.js- Compiled production buildREADME.md- Complete feature documentationTesting Infrastructure (7 files):
test/standalone.js,test/local.js,test/quick.jstest/README.md- Testing guide.github/workflows/test-list.yml.github/workflows/test-table.yml.github/workflows/test-svg.yml💡 Usage Examples
List Mode (default - no changes needed):
Table Mode:
SVG Mode:
🔄 Backward Compatibility
📊 Implementation Details
12 Commits following Gitmoji + Conventional Commits:
📸 Visual Preview
SVG Output Example:
The generated SVG includes: