getChordDefinitions() needs work

[edonv]

At first, I wasn't getting anything from it, but I think it's just that the parser is expecting define/chord directives in too specific a format.

It doesn't work if...

• base-fret is omitted, or
• any of fingers are anything other than a single-digit number above 0.
  • (also fails for any characters that should be useable for strings without a finger like N, x, -)

[martijnversluis]

Hey @edonv. Thanks for reaching out!

The implementation is based on the chordpro docs. This definition requires base-fret always to be defined. Are there apps that allow chord definitions without base-fret?

Besides the base fret, do you have some examples of definitions in chord sheets that are not or incorrectly parsed?

[edonv]

Hmmm, I guess it does say imply that base-fret is optional. I have a ton of sheets that use define that omit base-fret and ChordPro defaults base-fret to 1, so I guess I figured it was optional after all.

Regarding ones that don't work:

{define: D7 base-fret 1 frets N 3 2 3 1 N fingers a 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers A 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers x 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers X 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers n 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers N 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers - 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers -1 2 3 4 5 6}
{define: D7 base-fret 1 frets N 3 2 3 1 N fingers 0 2 3 4 5 6}

And I tried those after finding that {define: D7 base-fret 1 frets N 3 2 3 1 N fingers 1 2 3 4 5 6} is parsed correctly. fingers is supposed to support a few different values to indicate a string without a finger, specifically:

Finger settings may be numeric (0 .. 9) or uppercase letters (A .. Z). Note that the values -, x, X, and N are used to designate a string without finger setting.
As of now, ChordSheetJS doesn't support any type of non-numerical value for fingers (and doesn't support 0).

[martijnversluis]

@edonv You're right, the grammar is too simple right now.

Regarding your examples: from the docs I understand A-Z can be used as finger setting. I can imagine the grammar being case insensitive about all letters. But I really can't find examples of -1 being used as finger setting (in documentation). Is that used often?

[edonv]

Oh lol I should've clarified that I only tried -1 because I was curious if the parser would've worked with a negative number or only positive one. Yeah, that's not really a thing that's used for fingers.

[martijnversluis]

@edonv Thanks for reporting! I just published 10.8.1 which should improve the parsing.

Please let me know if that correctly solves your issue 👍

[edonv]

Thanks for this as well! Here's my update:

• Would you be able to make base-fret optional? Like I said, in my experience, it's supported by ChordPro itself.
• I'm noticing it having issues parsing chords with a name that has a slash in it. I'm not an expert by any means when it comes to PEG.js, but from what I can see, ChordDefinitionValue.name only supports alphanumeric characters. This led me to try other characters (like + or °/º or -), which all caused the chord to not be parsed. I think it'd be best if ChordDefinitionValue.name could support any characters until first whitespace ([^\s]).

[martijnversluis]

Hey @edonv. I just published 10.10.0 which makes base-fret optional and allows more special characters in the definition name. Please let me know if that is helpful 👍

[edonv]

Thanks! So it's nearly there. Characters like / and + work now, but # and - still aren't recognized.

Looking at the new ChordDefinitionName portion of the pattern, maybe break it into things like NoteName (note name and optional accidental), ChordQuality (major/minor/etc), ChordBassNote (/F#, slash + NoteName).

Pattern for each can be:

• NoteName: ([A-Ga-g](?:[b#♭♯]|es|s|is)?)
• ChordQuality: (.+)
  • not worth the effort to list out all possible chord qualities, also leaves it open-ended
  • this portion is optional
• ChordBassNote: (\/([A-Ga-g](?:[b#♭♯]|es|s|is)?))
  • a slash + same pattern from NoteName
  • this portion is optional too

This is obviously not all required, but the point is that the chord definition is made of 3 portions: the root note, the optional quality (m, maj7, 7, dim7, etc.), and an optional alternate bass note (/F#).

[martijnversluis]

@edonv I just published 10.10.1. This version should cover all possible suffixes 👍

[edonv]

Yes!! Sorry for all the fuss about it, this is perfect! I so appreciate all the hard work you put into this library!! Feel free to close the issue if there's nothing else to work on here.