Make renderChord() use render key modifier (#1535)

Author: martijnversluis

src/chord_sheet/song.ts

@@ -252,18 +252,33 @@ class Song extends MetadataAccessors {
    * @param {boolean} [options.normalizeChordSuffix=false] whether to normalize the chord suffixes after transposing
    * @returns {Song} The transposed song
    */
-  transpose(delta: number, { normalizeChordSuffix = false } = {}): Song {
+  transpose(
+    delta: number,
+    { modifier, normalizeChordSuffix = false }:
+      { modifier?: Modifier | null, normalizeChordSuffix?: boolean } = {},
+  ): Song {
     let transposedKey: Key | null = null;
     const song = (this as Song);
 
     return song.mapItems((item) => {
       if (item instanceof Tag && item.name === KEY) {
         transposedKey = Key.wrapOrFail(item.value).transpose(delta);
+
+        if (modifier) {
+          transposedKey = transposedKey.useModifier(modifier);
+        }
+
         return item.set({ value: transposedKey.toString() });
       }
 
       if (item instanceof ChordLyricsPair) {
-        return item.transpose(delta, transposedKey, { normalizeChordSuffix });
+        let chord = item.transpose(delta, transposedKey, { normalizeChordSuffix });
+
+        if (modifier) {
+          chord = chord.useModifier(modifier);
+        }
+
+        return chord;
       }
 
       return item;
@@ -308,7 +323,7 @@ class Song extends MetadataAccessors {
     const currentKey = this.requireCurrentKey();
     const targetKey = Key.wrapOrFail(newKey);
     const delta = currentKey.distanceTo(targetKey);
-    const transposedSong = this.transpose(delta);
+    const transposedSong = this.transpose(delta, { modifier: targetKey.modifier });
 
     if (targetKey.modifier) {
       return transposedSong.useModifier(targetKey.modifier);

src/helpers.ts

@@ -5,6 +5,7 @@ import Song from './chord_sheet/song';
 import { CAPO, CHORD_STYLE, ChordType } from './chord_sheet/tag';
 import Line from './chord_sheet/line';
 import FormattingContext from './formatter/formatting_context';
+import { Modifier } from './constants';
 
 export function transposeDistance(transposeKey: string, songKey: string): number {
   if (/^\d+$/.test(transposeKey)) {
@@ -61,6 +62,42 @@ interface RenderChordOptions {
   decapo?: boolean;
 }
 
+function effectiveModifier(renderKey: Key | null, contextKey: Key | null): Modifier | null {
+  if (renderKey?.modifier) {
+    return renderKey.modifier;
+  }
+
+  if (contextKey?.modifier) {
+    return contextKey.modifier;
+  }
+
+  return null;
+}
+
+function shapeChord(
+  {
+    chord,
+    effectiveTransposeDistance,
+    modifier,
+    normalizeChords,
+    effectiveKey,
+    chordStyle,
+  } : {
+    chord: Chord,
+    effectiveTransposeDistance: number,
+    modifier: Modifier | null,
+    normalizeChords: boolean,
+    effectiveKey: Key | null,
+    chordStyle: ChordType,
+  },
+) {
+  const transposedChord = chord.transpose(effectiveTransposeDistance);
+  const correctedChord = modifier ? transposedChord.useModifier(modifier) : transposedChord;
+  const normalizedChord = (normalizeChords ? correctedChord.normalize(effectiveKey) : transposedChord);
+
+  return changeChordType(normalizedChord, chordStyle, effectiveKey);
+}
+
 /**
  * Renders a chord in the context of a line and song and taking into account some options
  * @param chordString The chord to render
@@ -85,21 +122,30 @@ export function renderChord(
   }: RenderChordOptions = {},
 ): string {
   const chord = Chord.parse(chordString);
-  const songKey = song.key;
-  const capoString = song.metadata.getSingle(CAPO);
-  const capo = (decapo && capoString) ? parseInt(capoString, 10) : null;
-  const chordStyle = song.metadata.getSingle(CHORD_STYLE) as ChordType;
 
   if (!chord) {
     return chordString;
   }
 
-  const effectiveTransposeDistance = chordTransposeDistance(capo, line.transposeKey, songKey, renderKey);
-  const effectiveKey = renderKey || Key.wrap(line.key || song.key)?.transpose(effectiveTransposeDistance) || null;
-  const transposedChord = chord.transpose(effectiveTransposeDistance);
-  const normalizedChord = (normalizeChords ? transposedChord.normalize(effectiveKey) : transposedChord);
+  const songKey = song.key;
+  const capoString = song.metadata.getSingle(CAPO);
+  const capo = (decapo && capoString) ? parseInt(capoString, 10) : null;
+  const chordStyle = song.metadata.getSingle(CHORD_STYLE) as ChordType;
 
-  return changeChordType(normalizedChord, chordStyle, effectiveKey).toString({ useUnicodeModifier });
+  const effectiveTransposeDistance = chordTransposeDistance(capo, line.transposeKey, songKey, renderKey);
+  const contextKey = Key.wrap(line.key || song.key);
+  const modifier = effectiveModifier(renderKey, contextKey);
+  const effectiveKey = renderKey || contextKey?.transpose(effectiveTransposeDistance) || null;
+
+  return shapeChord({
+    chord,
+    effectiveTransposeDistance,
+    modifier,
+    normalizeChords,
+    effectiveKey,
+    chordStyle,
+  })
+    .toString({ useUnicodeModifier });
 }
 
 /**

test/helpers/render_chord.test.ts

@@ -36,7 +36,7 @@ describe('renderChord helper', () => {
       25 | "G"     | 3    |         | "A"              | "F"       | true   | "C#m7"  |
       26 | "G"     | 3    | "Bb"    |                  | "F"       | true   | "Bm7"   |
       27 | "G"     | 3    | "Bb"    | "A"              |           | true   | "Ebm7"  |
-      28 | "G"     | 3    | "Bb"    | "A"              | "F"       | true   | "C#m7"  |
+      28 | "G"     | 3    | "Bb"    | "A"              | "F"       | true   | "Dbm7"  |
       29 |         | 3    |         |                  |           | false  | "Em7"   |
       30 |         | 3    |         |                  | "F"       | false  | "Em7"   |
       31 |         | 3    |         | "A"              |           | false  | "Em7"   |
@@ -45,7 +45,7 @@ describe('renderChord helper', () => {
       34 |         | 3    | "Bb"    |                  | "F"       | false  | "Em7"   |
       35 |         | 3    | "Bb"    | "A"              |           | false  | "Em7"   |
       36 |         | 3    | "Bb"    | "A"              | "F"       | false  | "Em7"   |
-      37 | "G"     | 3    |         |                  |           | false  | "Em7"  |
+      37 | "G"     | 3    |         |                  |           | false  | "Em7"   |
       38 | "G"     | 3    |         |                  | "F"       | false  | "Dm7"   |
       39 | "G"     | 3    |         | "A"              |           | false  | "F#m7"  |
       40 | "G"     | 3    |         | "A"              | "F"       | false  | "Em7"   |
@@ -66,6 +66,34 @@ describe('renderChord helper', () => {
       const renderedChord = renderChord('Em7', line, song, { renderKey: Key.wrap(renderKey), decapo });
       expect(renderedChord).toEqual(outcome);
     });
+
+    it('respects a higher # rendering key', () => {
+      const song = new Song();
+      song.metadata.add('key', 'A');
+      const renderedChord = renderChord('A', new Line(), song, { renderKey: Key.parse('A#') });
+      expect(renderedChord).toEqual('A#');
+    });
+
+    it('respects a lower # rendering key', () => {
+      const song = new Song();
+      song.metadata.add('key', 'A');
+      const renderedChord = renderChord('A', new Line(), song, { renderKey: Key.parse('G#') });
+      expect(renderedChord).toEqual('G#');
+    });
+
+    it('respects a higher b rendering key', () => {
+      const song = new Song();
+      song.metadata.add('key', 'A');
+      const renderedChord = renderChord('A', new Line(), song, { renderKey: Key.parse('Bb') });
+      expect(renderedChord).toEqual('Bb');
+    });
+
+    it('respects a lower b rendering key', () => {
+      const song = new Song();
+      song.metadata.add('key', 'A');
+      const renderedChord = renderChord('A', new Line(), song, { renderKey: Key.parse('Ab') });
+      expect(renderedChord).toEqual('Ab');
+    });
   });
 
   describe('chord transposition solfege', () => {
@@ -99,7 +127,7 @@ describe('renderChord helper', () => {
       25 | "Sol"     | 3    |         | "La"             | "Fa"      | true   | "Do#m7"  |
       26 | "Sol"     | 3    | "Sib"   |                  | "Fa"      | true   | "Sim7"   |
       27 | "Sol"     | 3    | "Sib"   | "La"             |           | true   | "Mibm7"  |
-      28 | "Sol"     | 3    | "Sib"   | "La"             | "Fa"      | true   | "Do#m7"  |
+      28 | "Sol"     | 3    | "Sib"   | "La"             | "Fa"      | true   | "Rebm7"  |
       29 |           | 3    |         |                  |           | false  | "Mim7"   |
       30 |           | 3    |         |                  | "Fa"      | false  | "Mim7"   |
       31 |           | 3    |         | "La"             |           | false  | "Mim7"   |