bettermusic / ChordSheetJS

A JavaScript library for parsing and formatting ChordPro chord sheets
GNU General Public License v2.0
4 stars 1 forks source link

💡 RFC: Pass metadata as it's own param to a parser, allow it to be hidden in formatters. #215

Closed isaiahdahl closed 1 year ago

isaiahdahl commented 2 years ago

Parent Task: https://github.com/PraiseCharts/studio/issues/32

Background & Motivation

In it's simplest form. We need to have the ability to edit chordpro & chords over words without showing any metadata, or allowing metadata to be updated.

Obviously some metadata needs to be updated to power the song, like key & capo. but if there was no way to edit the metadata in the actual text editor portion, they could be updated with the relevant buttons hooked up to functions on the song.

In theory, the scope of this task could be expanded for more granular control over certain locked metadata's.... like allowing key and capo metadata to be parsed but not others, but It would satisfy the objective perfectly if we just had a way to keep all the metadata separate from the text it's parsing, ie. ChordPro or a Text Chord sheet, and then make sure that there are functions that can do the jobs needed ie: setKey(), setCap0() ....etc.

Proposed Solution

Pass metadata as a secondary param to the parsers.

const chordpro = `...`
const metadata = state.metadata // some object containing all metadata you want to pass in
const parser = new ChordProParser();
const song = parser.parse(chordpro, metadata) 

Metadata would be optional, but if it's passed in, it would take presedence over what's parsed from the chordpro.

We could then add a configurable option to the relevant formatters to hide the metadata. ChordProFormatter() & TextFormatter() assuming we ended up adding some way to pass metadata in a text chord sheet

Then it's the job of the implementation to hold the metadata state, and continue passing it into parser.

isaiahdahl commented 2 years ago

@martijnversluis Not sure if you've seen this one yet or not. But I feel like it'd be a pretty straightforward win.

martijnversluis commented 2 years ago

I’d seen it, but i had to read it again today to fully understand your explanation.

I think I’d keep that out of the parsers, something like:

const song = new SomeParser().parse(sheet);
song.metadata = song.metadata.merge(state.metadata);

What do you think?

isaiahdahl commented 2 years ago

That sounds good to me. Easier to share between all parsers too!