-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Implement OnSong parser #461
base: master
Are you sure you want to change the base?
Conversation
f96211a
to
fa6045d
Compare
DirectiveSection | ||
= "{" _ ("start_of_" / "so") type:DirectiveSectionTypes _ name:(":" _ @MetaValue)? "}" EOL | ||
items:SectionBody* | ||
"{" _ ("end_of_" / "eo") DirectiveSectionTypes _ "}" EOL // FIXME: must match start tag |
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.
FIXME found
src/parser/on_song_grammar.pegjs
Outdated
ChordsOverLyrics | ||
= chords:(_ @(chord:Chord { return { chord, column: location().start.column } }))+ EOL | ||
lyrics:(@Lyrics EOL)? { | ||
// FIXME: |
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.
FIXME found
'[D/F#]': 'D/F#', | ||
'[Bsus2]': 'Bsus2', | ||
'\\[notachord]': Error, | ||
// '[unknown]': ExpectWarning, // FIXME |
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.
FIXME found
* origin/master: Bump jest from 27.4.0 to 27.5.1 (martijnversluis#463) Bump @babel/core from 7.17.0 to 7.17.5 (martijnversluis#464) 6.0.2 Ensure pinst is installed 6.0.1 Add Husky pre-commit hook to re-generate README.md on commit (martijnversluis#462) Point sponsor button to GitHub sponsors profile 6.0.0 Enharmonic rules (martijnversluis#460)
], | ||
}, | ||
|
||
// TODO: "You can also start the line with a period or a back tick character |
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.
TODO found
ChordsOverLyrics | ||
= annotations:(_ @(annotation:(Chord / MusicalInstruction) { return { annotation, column: location().start.column } }))+ EOL | ||
lyrics:(@Lyrics EOL)? { | ||
// FIXME: Is there a better way to do this in PEG? |
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.
FIXME found
* origin/master: Bump eslint from 8.9.0 to 8.10.0 (martijnversluis#468) 6.1.0 Implement Song#setCapo and Song#setKey (martijnversluis#467) Bump pinst from 2.1.6 to 3.0.0 (martijnversluis#465) Bump @babel/cli from 7.17.3 to 7.17.6 (martijnversluis#466)
Code Climate has analyzed commit d32c941 and detected 4 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
@bkeepers Hey just wanted to notify you: within a downstream fork we did a TypeScript conversion and we had to migrate to Peggy too. I'll be merging the PR. Please let me know if you need any help rebasing your PR, I'm eager to help you out. |
This is a WIP for implementing a parser for the OnSong format. I'm opening it early as it will likely be a long-lived PR that will require a lot of feedback and involve changes to other parts of ChordSheetJS.
TODO:
{start_of_*}…{end_of_*}
)Song
modelMaybe not in this PR, but other things that could be added eventually:
parse(source, {transpose: 2})
- With support for calling JS functions in peg.js, I think it would be just as easy to perform transpose at parse timeCloses #458
cc chordbook/chordbook#130 @mattgraham