Tuning constructor defaults to 8 strings?

[ronyeh]

https://github.com/0xfe/vexflow/blob/a3ac41ef4041d5d321d10eaf96b1b52857349b58/src/tuning.ts#L25-L28

The default tuningString is E/5,B/4,G/4,D/4,A/3,E/3,B/2,E/2 which looks like the shape of a standard tuning of a 6-string guitar but with two extra bass strings at B/2 and E/2. Is this a bug? The comment suggests it should default to standard tuning.

Additionally, the guitar tunings seem to be one octave higher than I expect.

https://github.com/0xfe/vexflow/blob/a3ac41ef4041d5d321d10eaf96b1b52857349b58/src/tuning.ts#L11-L19

Shouldn't standard tuning go from E/2 to E/4?

C/4 is on string 2, fret 1 of a 6-string guitar with standard tuning.

I'm in the middle of cleaning up the migration/tests PR and I found what I thought was an incorrect test case comment:

https://github.com/0xfe/vexflow/blob/a3ac41ef4041d5d321d10eaf96b1b52857349b58/tests/tuning_tests.ts#L23-L27

When I change the 9 to a 7 to match the comment, the test case fails.... since the standard tuning in VexFlow actually has 8 strings!

Additionally, the .getValueForString() method doesn't do what I initially expected:

https://github.com/0xfe/vexflow/blob/a3ac41ef4041d5d321d10eaf96b1b52857349b58/tests/tuning_tests.ts#L29-L34

The values are set by:

https://github.com/0xfe/vexflow/blob/a3ac41ef4041d5d321d10eaf96b1b52857349b58/src/tuning.ts#L30-L33

It looks like C/5 is mapped to value 60, so my assumption is that this is using the MIDI convention of # 60 == Middle C.

It is also assuming that C/5 == Middle C, so that would make the octave numbers of the "standard tuning" make more sense.

However, in other parts of VexFlow, C/4 is middle C, which corresponds with scientific pitch notation.

In summary, Tuning probably has a small bug in the constructor, and it either needs extra comments to explain that middle C == C/5 == note value 60, or it needs to somehow be reworked such that middle C is C/4, like in StaveNote.