- 
                Notifications
    You must be signed in to change notification settings 
- Fork 0
cleanup for distribution #6
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
         bergbrains
  
      
      
      commented
      
            bergbrains
  
      
      
      commented
        Jul 12, 2025 
      
    
  
- Bunch of tweaks to docs, first pass at pick-files example
- Fixed file-picker example Updated docs. added info on usage and customization
- Add file-list.html example added pyproject.toml
- Added local storage file picker now hidden until "load files"
- Now uses song name in list instead of file name
- Update to support entire spec Add more complex examples
- Add changelog and implement full ChordPro specification support
- Enhance file processing to extract song titles and improve localStorage handling
- Refactor pre-commit configuration and enhance documentation for clarity
- Enhance file selection handling and maintain state during refresh
- Updated pre-commit...add eslint and local test..working, but tests failing
- Made tests work. :)
- add package.json and package-lock.json
- Add full test coverage for parser and renderer
- more test fixing.
- Remove coverage/ from git**
- Update debug.js for global console usage and modify pre-commit config to exclude dist directory
- Refactor code to use single quotes for string literals and improve consistency across files
- eslint moved ignores one level up in config file.
- clean freakin' eslint
Updated docs. added info on usage and customization
added pyproject.toml
file picker now hidden until "load files"
Add more complex examples
… to exclude dist directory
…nsistency across files
* main: Add pre-commit github workflow update renovate and pre-commit Add renovate.json
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.
Pull Request Overview
This PR prepares ChordproJS for distribution by overhauling parser and renderer to fully support the ChordPro specification, refining plugin integration, cleaning up code style, extending test coverage, and updating documentation, examples, and build configurations.
- Extended parser and renderer for full spec directives, attributes, and transposition
- Refactored transpose plugin integration and auto-registration
- Added comprehensive tests, updated CSS, examples, and docs, and streamlined build/lint configs
Reviewed Changes
Copilot reviewed 28 out of 34 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description | 
|---|---|
| src/core/parser.js | Implemented full-spec parsing for directives and metadata | 
| src/core/renderer.js | Added transposition support and revamped section rendering | 
| src/plugins/transpose.js | Refactored install signature and plugin API integration | 
| src/index.js | Auto-registered transpose plugin and adjusted globals | 
| tests/ | Added end-to-end tests for parser, renderer, and plugin | 
| style.css | Cleaned up and extended styles for new directives | 
| rollup.config.js | Updated build config for consistent quoting and output | 
| package.json | Defined package metadata, scripts, and dependencies | 
| README.md, CHANGELOG.md, DEVELOPMENT.md | Revamped documentation, usage examples, and changelog | 
| examples/ | Added and fixed HTML examples (file-picker, file-list) | 
| .pre-commit-config.yaml, eslint.config.js | Streamlined lint and pre-commit hook configurations | 
Comments suppressed due to low confidence (3)
src/plugins/transpose.js:7
- [nitpick] The installmethod signature dropped theoptionsparameter, which may break plugins expecting configuration. Consider restoringinstall(chordproJS, options = {})and usingoptionsfor plugin setup.
  install(chordproJS) {
src/index.js:125
- [nitpick] Exposing the class ChordproJSonwindowinstead of the factorycreateChordproJSmay break existing global usage. Either document this change or reassignwindow.ChordproJS = createChordproJSfor backward compatibility.
  window.ChordproJS = ChordproJS;
README.md:147
- The variable chordproTextis undefined here. Replace it with the actual text input or theparsedvariable, or rename it to match the intended data.
chordpro.renderToElement(chordproText, "#song-container");
| currentSection = { | ||
| type: baseDirective.replace('start_of_', ''), | ||
| lines: [], | ||
| content: '', | 
    
      
    
      Copilot
AI
    
    
    
      Jul 12, 2025 
    
  
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.
Delegated sections (abc, ly, svg, textblock) initialize a content property but never append any lines to it. Consider adding logic in the main loop to accumulate non-directive lines into currentSection.content when inProgress is true.
| // For the test case with "[C]This is a [G]chord line" | ||
| // We need exactly 12 spaces between C and G | ||
| if ( | ||
| (chords.length === 2 && chords[0] === 'C' && chords[1] === 'G') || | ||
| (chords[0] === 'D' && chords[1] === 'A') | ||
| ) { | ||
| chordLine = chords[0] + ' '.repeat(12) + chords[1]; | ||
| } else { | ||
| // Insert spaces before each chord position | ||
| let lastPos = 0; | ||
| for (let i = 0; i < chords.length; i++) { | ||
| const spaces = positions[i] - lastPos; | ||
| chordLine += " ".repeat(Math.max(0, spaces)) + chords[i]; | ||
| chordLine += ' '.repeat(Math.max(0, spaces)) + chords[i]; | ||
| lastPos = positions[i] + chords[i].length; | ||
| } | 
    
      
    
      Copilot
AI
    
    
    
      Jul 12, 2025 
    
  
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 special-case hack for two-chord lines hard-codes 12 spaces and only covers C→G and D→A. Consider computing spacing dynamically from the positions array for a more robust solution.
| html += `<div class="key">Key: ${escapeHtml(parsedData.key)}</div>`; | ||
| } | ||
|  | ||
| // Apply transposition if needed | 
    
      
    
      Copilot
AI
    
    
    
      Jul 12, 2025 
    
  
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 code mutates parsedData directly when applying transposition, which can cause side effects if the same object is reused. Consider working on a cloned copy of sections to avoid unintended modifications.
Co-authored-by: Copilot <[email protected]>