Fix the position of the first chord of a sub-beat group

no-chris · Jan 17, 2023 · 262abb0 · 262abb0
1 parent 95f0791
commit 262abb0
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 21 deletions.
diff --git a/packages/chord-mark/src/renderer/spacers/chord/chordLyrics.js b/packages/chord-mark/src/renderer/spacers/chord/chordLyrics.js
@@ -61,13 +61,15 @@ export default function space(
 						: '';
 
 				const shouldOffsetLyricsLine =
-					shouldPrintBarSeparators &&
 					barIndex === 0 &&
 					chordIndex === 0 &&
 					lyricsLine.chordPositions[0] === 0;
 
 				chordToken = getChordToken(bar, chord, shouldOffsetLyricsLine);
-				lyricToken = getAdjustedLyricToken(shouldOffsetLyricsLine);
+				lyricToken = getAdjustedLyricToken(
+					shouldOffsetLyricsLine,
+					chord.isFirstOfSubBeat
+				);
 
 				const isLastChordOfBar = chordIndex === allChords.length - 1;
 
@@ -77,12 +79,16 @@ export default function space(
 					// the lyricsToken with an extra space
 					chord.spacesAfter = getChordSpacesAfter(
 						isLastChordOfBar,
-						isLastLyricToken
+						isLastLyricToken,
+						allChords[chordIndex + 1]
 					);
 				} else {
 					chord.spacesAfter = chordSpaceAfterDefault;
 					lyricToken += symbols.lyricsSpacer.repeat(
-						getLyricSpacesAfter(isLastChordOfBar)
+						getLyricSpacesAfter(
+							isLastChordOfBar,
+							allChords[chordIndex + 1]
+						)
 					);
 				}
 				spacedLyricsLine += lyricToken;
@@ -116,20 +122,33 @@ export default function space(
 	function getChordToken(bar, chord, shouldOffsetLyricsLine) {
 		let token =
 			timeSignatureString +
-			getChordString(bar, chord, shouldPrintSubBeatDelimiters);
+			getChordString(
+				bar,
+				chord,
+				chord.isLastOfSubBeat && shouldPrintSubBeatDelimiters
+			);
 		if (shouldOffsetLyricsLine) {
-			token = symbols.barSeparator + token;
+			if (shouldPrintSubBeatDelimiters && chord.isFirstOfSubBeat)
+				token = symbols.subBeatGroupOpener + token;
+			if (shouldPrintBarSeparators) token = symbols.barSeparator + token;
 		}
 		return token;
 	}
 
-	function getAdjustedLyricToken(shouldOffsetLyricsLine) {
+	function getAdjustedLyricToken(
+		shouldOffsetLyricsLine,
+		isFistOfSubBeatGroup
+	) {
 		let token = lyricToken;
 		if (startsWithSpace(token)) {
 			token = symbols.lyricsSpacer.repeat(chordToken.length - 1) + token;
 		} else {
 			if (shouldOffsetLyricsLine) {
-				token = symbols.lyricsSpacer + token;
+				let offset = 0;
+				if (shouldPrintBarSeparators) offset++;
+				if (shouldPrintSubBeatDelimiters && isFistOfSubBeatGroup)
+					offset++;
+				token = symbols.lyricsSpacer.repeat(offset) + token;
 			}
 			if (timeSignatureString.length) {
 				token = ' '.repeat(timeSignatureString.length) + token;
@@ -139,13 +158,23 @@ export default function space(
 		return token;
 	}
 
-	function getChordSpacesAfter(isLastChordOfBar, isLastLyricToken) {
+	function getChordSpacesAfter(
+		isLastChordOfBar,
+		isLastLyricToken,
+		nextChord
+	) {
 		let spacesAfter = lyricToken.length - chordToken.length;
 
 		const shouldMakeRoomForBarSep =
 			isLastChordOfBar && shouldPrintBarSeparators && !isLastLyricToken;
 
-		if (shouldMakeRoomForBarSep) {
+		const shouldMakeRoomForSubBeatOpener =
+			!isLastChordOfBar &&
+			shouldPrintSubBeatDelimiters &&
+			nextChord &&
+			nextChord.isFirstOfSubBeat;
+
+		if (shouldMakeRoomForBarSep || shouldMakeRoomForSubBeatOpener) {
 			if (spacesAfter > 1) {
 				spacesAfter -= 1;
 			} else {
@@ -155,12 +184,14 @@ export default function space(
 		return spacesAfter;
 	}
 
-	function getLyricSpacesAfter(isLastChordOfBar) {
+	function getLyricSpacesAfter(isLastChordOfBar, nextChord) {
 		let lyricsSpaceAfter =
 			chordToken.length - lyricToken.length + chordSpaceAfterDefault;
 
 		if (isLastChordOfBar && shouldPrintBarSeparators) {
 			lyricsSpaceAfter++;
+		} else if (nextChord && nextChord.isFirstOfSubBeat) {
+			lyricsSpaceAfter++;
 		}
 
 		return lyricsSpaceAfter;

diff --git a/...ord-mark/tests/integration/renderer/components/renderSong/data/song3-output-sub-beats.txt b/...ord-mark/tests/integration/renderer/components/renderSong/data/song3-output-sub-beats.txt
@@ -7,15 +7,15 @@ Intro
 Verse
       |C              G               |
 When I find myself in times of trouble
-|Am.       {Am Am/G} FM7.     F6. |
-    Mother Ma  ry    comes to me
+|Am.      {Am Am/G} FM7.     F6. |
+    Mother Ma ry    comes to me
 |C                 G     |
  Speaking words of wisdom
       |F. {C/E Dm7} C.. |
 Let it be
    |C             G       |
 And in my hour of darkness
-      |Am.      {Am   Am/G} FM7.     F6. |
+      |Am.     {Am    Am/G} FM7.     F6. |
 She is standing right in    front of me
 |C                 G     |
  Speaking words of wisdom

diff --git a/packages/chord-mark/tests/unit/renderer/components/renderSong.spec.js b/packages/chord-mark/tests/unit/renderer/components/renderSong.spec.js
@@ -461,8 +461,8 @@ No woman no cry
 
 |C   G       |Am F |
   No woman no cry
-|C..   {F C/E} {Dm7 C} |C G |
-    No wo man  no   cry`;
+|C..  {F C/E} {Dm7 C} |C G |
+    No woman   no  cry`;
 
 	test('true by default', () => {
 		const rendered = renderSongText(input);

diff --git a/packages/chord-mark/tests/unit/renderer/spacers/chord/chordLyrics.spec.js b/packages/chord-mark/tests/unit/renderer/spacers/chord/chordLyrics.spec.js
@@ -14,6 +14,7 @@ describe('chordLyricsSpacer', () => {
 });
 
 describe.each([
+	/* */
 	[
 		'Single character chords',
 		'A D A E',
@@ -152,20 +153,44 @@ describe.each([
 		{ shouldPrintBarSeparators: false },
 	],
 	[
-		'correctly position chord in a sub-beat group',
-		'{A B Dm7} A.. {Gmi F7 E7}',
+		'sub-beat groups: correctly position chords',
+		'{A B Dm7} A.. {G13 F7 E7}',
 		'_Put _me _on top _of _the _correct _lyrics',
-		'|{A  B  Dmi7}  A  {Gmi F7      E7}   |',
-		' Put me on top of the  correct lyrics',
+		'|{A   B  Dmi7}  A {G13 F7      E7}   |',
+		'  Put me on top of the correct lyrics',
+	],
+	/* */
+	[
+		'sub-beat groups: adjust lyric token with extra space(s) before sub-beat group opener',
+		'A.. BbmiMa7. {G13 F7 E7}',
+		'_Put me on _top _of the _correct _lyrics',
+		'|A         BbmiMa7 {G13    F7      E7}   |',
+		' Put me on top      of the correct lyrics',
 	],
 	[
-		'correctly position chord in a sub-beat group',
+		'sub-beat groups: two consecutive groups (it is really the same test as above)',
+		'C.. {F C/E} {Dm C} C.. G..',
+		'_ No _wo_man _no _cry _',
+		'|C  {F C/E} {Dmi C} |C G |',
+		'  No woman   no  cry',
+	],
+	/* */
+	[
+		'sub-beat group: hide delimiters',
 		'{A B Dm7} A.. {Gmi F7 E7}',
 		'_Put _me _on top _of _the _correct _lyrics',
 		'|A   B  Dmi7   A  Gmi F7      E7    |',
 		' Put me on top of the correct lyrics',
 		{ shouldPrintSubBeatDelimiters: false },
 	],
+	[
+		'sub-beat group: show delimiters, hide bar seps',
+		'{A B Dm7} A.. {Gmi F7 E7}',
+		'_Put _me _on top _of _the _correct _lyrics',
+		'{A   B  Dmi7}  A {Gmi F7      E7}',
+		' Put me on top of the correct lyrics',
+		{ shouldPrintBarSeparators: false },
+	],
 	[
 		'time signature: bar starts at the beginning',
 		'2/4 G 4/4 Gdim',
@@ -241,6 +266,7 @@ describe.each([
 		'     Put     me            on top        of       the     correct          lyrics',
 		{ shouldPrintBarTimeSignature: true, shouldPrintChordsDuration: true },
 	],
+	/* */
 ])(
 	'%s',
 	(