Skip to content

Commit

Permalink
Merge pull request #655 from no-chris/fix-unwanted-auto-tranpose
Browse files Browse the repository at this point in the history
Fix unwanted auto tranpose
  • Loading branch information
no-chris committed Jan 25, 2024
2 parents d338c3e + f38d6bf commit e9c4a4a
Show file tree
Hide file tree
Showing 10 changed files with 109 additions and 18 deletions.
8 changes: 5 additions & 3 deletions packages/chord-mark/src/parser/parseChordLine.js
Expand Up @@ -28,6 +28,7 @@ const barRepeatSymbols = new RegExp(
* @typedef {Object} ChordLine
* @type {Object}
* @property {Bar[]} allBars
* @property {KeyDeclaration} originalKey
* @property {Boolean} hasPositionedChords
*/

Expand Down Expand Up @@ -60,12 +61,12 @@ const barRepeatSymbols = new RegExp(
* @param {String} chordLine
* @param {Object} options
* @param {TimeSignature} options.timeSignature
* @param {KeyDeclaration} options.key
* @param {KeyDeclaration} options.originalKey
* @returns {ChordLine}
*/
export default function parseChordLine(
chordLine,
{ timeSignature = defaultTimeSignature, key = {} } = {}
{ timeSignature = defaultTimeSignature, originalKey = {} } = {}
) {
let { beatCount } = timeSignature;

Expand Down Expand Up @@ -111,6 +112,7 @@ export default function parseChordLine(

return {
allBars,
originalKey,
};

function repeatPreviousBars(token) {
Expand Down Expand Up @@ -151,7 +153,7 @@ export default function parseChordLine(
duration: getChordDuration(token, beatCount, isInSubBeatGroup),
model: isNoChordSymbol(cleanedToken)
? syntax.noChord
: parseChord(cleanedToken, key),
: parseChord(cleanedToken, originalKey),
beat: currentBeatCount + 1,
isInSubBeatGroup,
};
Expand Down
4 changes: 2 additions & 2 deletions packages/chord-mark/src/parser/songLinesFactory.js
Expand Up @@ -166,7 +166,7 @@ export default function songLinesFactory() {
try {
const model = parseChordLine(string, {
timeSignature: currentTimeSignature,
key: currentKey,
originalKey: currentKey,
});
line = {
string,
Expand Down Expand Up @@ -302,7 +302,7 @@ export default function songLinesFactory() {

const currentSectionContent = remainingLines
.slice(0, nextSectionIndex !== -1 ? nextSectionIndex : undefined)
.filter((line) => !(isTimeSignature(line) || isEmptyLine(line)));
.filter((line) => !(isTimeSignature(line) || isEmptyLine(line))); //todo: add key definition line type as well

return currentSectionContent.length === 0;
}
Expand Down
12 changes: 9 additions & 3 deletions packages/chord-mark/src/renderer/helpers/renderAllChords.js
Expand Up @@ -60,10 +60,16 @@ export default function renderAllChords(
}

function shouldTransposeRepeatedChords(line) {
const currentKeyEqualsOriginalKey =
currentKey &&
line.model.originalKey &&
line.model.originalKey.string === currentKey.string;

return (
line.isFromAutoRepeatChords ||
line.isFromSectionCopy ||
line.isFromChordLineRepeater
(line.isFromAutoRepeatChords ||
line.isFromSectionCopy ||
line.isFromChordLineRepeater) &&
!currentKeyEqualsOriginalKey
);
}

Expand Down
16 changes: 16 additions & 0 deletions packages/chord-mark/tests/integration/parser/parseSong.spec.js
Expand Up @@ -65,6 +65,10 @@ key Am`;
lineHadTimeSignatureChange: false,
},
],
originalKey: {
string: 'G',
accidental: 'sharp',
},
hasPositionedChords: false,
},
},
Expand Down Expand Up @@ -144,6 +148,10 @@ key Am`;
lineHadTimeSignatureChange: false,
},
],
originalKey: {
string: 'G',
accidental: 'sharp',
},
hasPositionedChords: true,
},
},
Expand Down Expand Up @@ -186,6 +194,10 @@ key Am`;
lineHadTimeSignatureChange: false,
},
],
originalKey: {
string: 'G',
accidental: 'sharp',
},
hasPositionedChords: false,
},
},
Expand Down Expand Up @@ -251,6 +263,10 @@ key Am`;
lineHadTimeSignatureChange: false,
},
],
originalKey: {
string: 'G',
accidental: 'sharp',
},
hasPositionedChords: true,
},
},
Expand Down
44 changes: 40 additions & 4 deletions packages/chord-mark/tests/unit/parser/parseChordLine.spec.js
Expand Up @@ -55,6 +55,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -87,6 +88,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -126,6 +128,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -172,6 +175,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -212,6 +216,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -259,6 +264,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -313,6 +319,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -375,6 +382,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand All @@ -400,6 +408,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -432,6 +441,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -464,6 +474,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -503,6 +514,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -558,6 +570,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand All @@ -583,6 +596,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand All @@ -608,6 +622,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand All @@ -633,6 +648,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -665,6 +681,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -742,6 +759,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -812,6 +830,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -880,6 +899,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],

Expand Down Expand Up @@ -920,6 +940,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -962,6 +983,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1013,6 +1035,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1073,6 +1096,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1142,6 +1166,7 @@ describe.each([
lineHadTimeSignatureChange: false,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1188,6 +1213,7 @@ describe.each([
lineHadTimeSignatureChange: true,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1256,6 +1282,7 @@ describe.each([
lineHadTimeSignatureChange: true,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1324,6 +1351,7 @@ describe.each([
lineHadTimeSignatureChange: true,
},
],
originalKey: {},
},
],
[
Expand Down Expand Up @@ -1363,6 +1391,7 @@ describe.each([
lineHadTimeSignatureChange: true,
},
],
originalKey: {},
},
],
])('%s: %s', (title, input, timeSignature, expected) => {
Expand All @@ -1376,12 +1405,19 @@ describe.each([

describe('chord key', () => {
test('should forward the key to the chord parser', () => {
const options = { key: { string: 'C' } };
const options = { originalKey: { string: 'C' } };
parseChordLine('C F G', options);

expect(parseChord).toHaveBeenNthCalledWith(1, 'C', options.key);
expect(parseChord).toHaveBeenNthCalledWith(2, 'F', options.key);
expect(parseChord).toHaveBeenNthCalledWith(3, 'G', options.key);
expect(parseChord).toHaveBeenNthCalledWith(1, 'C', options.originalKey);
expect(parseChord).toHaveBeenNthCalledWith(2, 'F', options.originalKey);
expect(parseChord).toHaveBeenNthCalledWith(3, 'G', options.originalKey);
});

test('should save the original key as a property', () => {
const options = { originalKey: { string: 'C' } };
const parsed = parseChordLine('C F G', options);

expect(parsed.originalKey).toEqual(options.originalKey);
});
});

Expand Down
Expand Up @@ -143,17 +143,17 @@ describe('timeSignature', () => {
expect(parseChordLine.mock.calls[0][0]).toEqual('Em D. C.');
expect(parseChordLine.mock.calls[0][1]).toEqual({
timeSignature: ts6_8,
key: {},
originalKey: {},
});
expect(parseChordLine.mock.calls[1][0]).toEqual('C.. G..');
expect(parseChordLine.mock.calls[1][1]).toEqual({
timeSignature: ts4_4,
key: {},
originalKey: {},
});
expect(parseChordLine.mock.calls[2][0]).toEqual('D D C A');
expect(parseChordLine.mock.calls[2][1]).toEqual({
timeSignature: ts3_4,
key: {},
originalKey: {},
});
});
});
Expand Down

0 comments on commit e9c4a4a

Please sign in to comment.