Skip to content

Commit

Permalink
Fix disabling chord normalization in formatters (#1167)
Browse files Browse the repository at this point in the history
When instantiating a formatter with option `normalizeChords: false`, normalization should not be applied. This functionality was not implemented for `ChordProFormatter` and `ChordsOverWordsFormatter`.

Related to #1166
Thanks to @ldelia for reporting.
  • Loading branch information
martijnversluis authored May 10, 2024
1 parent c4ce11d commit 0188f3c
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 16 deletions.
10 changes: 9 additions & 1 deletion src/formatter/chord_pro_formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Item from '../chord_sheet/item';
import Evaluatable from '../chord_sheet/chord_pro/evaluatable';
import Comment from '../chord_sheet/comment';
import SoftLineBreak from '../chord_sheet/soft_line_break';
import Chord from '../chord';

/**
* Formats a song into a ChordPro chord sheet
Expand Down Expand Up @@ -137,7 +138,14 @@ class ChordProFormatter extends Formatter {

formatChordLyricsPairChords(chordLyricsPair: ChordLyricsPair): string {
if (chordLyricsPair.chords) {
return `[${chordLyricsPair.chords}]`;
const renderedChord = Chord.parse(chordLyricsPair.chords);

if (!renderedChord) {
return `[${chordLyricsPair.chords}]`;
}

const normalizedChord = this.configuration.normalizeChords ? renderedChord.normalize() : renderedChord;
return `[${normalizedChord}]`;
}

if (chordLyricsPair.annotation) {
Expand Down
23 changes: 14 additions & 9 deletions src/formatter/chords_over_words_formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,7 @@ class ChordsOverWordsFormatter extends Formatter {
}

chordLyricsPairLength(chordLyricsPair: ChordLyricsPair, line: Line): number {
const content = chordLyricsPair.annotation || renderChord(
chordLyricsPair.chords,
line,
this.song,
{ renderKey: this.configuration.key },
);

const content = chordLyricsPair.annotation || this.renderChord(chordLyricsPair, line);
const { lyrics } = chordLyricsPair;
const contentLength = (content || '').length;
const lyricsLength = (lyrics || '').length;
Expand All @@ -110,14 +104,25 @@ class ChordsOverWordsFormatter extends Formatter {
}

if (item instanceof ChordLyricsPair) {
const content = item.annotation
|| renderChord(item.chords, line, this.song, { renderKey: this.configuration.key });
const content = item.annotation || this.renderChord(item, line);
return padLeft(content, this.chordLyricsPairLength(item, line));
}

return '';
}

renderChord(item: ChordLyricsPair, line: Line) {
return renderChord(
item.chords,
line,
this.song,
{
renderKey: this.configuration.key,
normalizeChords: this.configuration.normalizeChords,
},
);
}

formatLineBottom(line, metadata) {
if (hasTextContents(line)) {
return this.formatLineWithFormatter(line, this.formatItemBottom, metadata);
Expand Down
21 changes: 21 additions & 0 deletions test/formatter/chord_pro_formatter.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { ChordProFormatter } from '../../src';
import { exampleSongSolfege, exampleSongSymbol } from '../fixtures/song';
import { chordProSheetSolfege, chordProSheetSymbol } from '../fixtures/chord_pro_sheet';
import { chordLyricsPair, createSongFromAst } from '../utilities';

describe('ChordProFormatter', () => {
it('formats a symbol song to a chord pro sheet correctly', () => {
Expand All @@ -10,4 +11,24 @@ describe('ChordProFormatter', () => {
it('formats a solfege song to a chord pro sheet correctly', () => {
expect(new ChordProFormatter().format(exampleSongSolfege)).toEqual(chordProSheetSolfege);
});

it('allows enabling chord normalization', () => {
const formatter = new ChordProFormatter({ normalizeChords: true });

const song = createSongFromAst([
[chordLyricsPair('Dsus4', 'Let it be')],
]);

expect(formatter.format(song)).toEqual('[Dsus]Let it be');
});

it('allows disabling chord normalization', () => {
const formatter = new ChordProFormatter({ normalizeChords: false });

const song = createSongFromAst([
[chordLyricsPair('Dsus4', 'Let it be')],
]);

expect(formatter.format(song)).toEqual('[Dsus4]Let it be');
});
});
15 changes: 15 additions & 0 deletions test/formatter/chords_over_words_formatter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
} from '../../src';

import {
chordLyricsPair,
createSongFromAst, heredoc, section,
} from '../utilities';

Expand Down Expand Up @@ -114,6 +115,20 @@ Grid line 2`;
expect(formatter.format(songWithIntro)).toEqual(expectedChordSheet);
});

it('allows to disable normalizing chords', () => {
const formatter = new ChordsOverWordsFormatter({ normalizeChords: false });

const song = createSongFromAst([
[chordLyricsPair('Dsus4', 'Let it be')],
]);

const expectedChordSheet = heredoc`
Dsus4
Let it be`;

expect(formatter.format(song)).toEqual(expectedChordSheet);
});

describe('delegates', () => {
[ABC, GRID, LILYPOND, TAB].forEach((type) => {
describe(`for ${type}`, () => {
Expand Down
4 changes: 2 additions & 2 deletions test/integration/chords_over_words_to_chordpro.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ describe('chords over words to chordpro', () => {
Let it [Am]be, let it [C/G]be, let it [F]be, let it [C]be
[C]Whisper words of [G]wisdom, let it [F]be[C/E][Dm][C]
Let it [Bbm7]be, let it [DbM7/Ab]be, let it [Gbsus2]be, let it [Db]be
[GbM7]Whisper words of [Absus]wisdom, let it [Gb]be[Db/F][Ebm][Db]`;
Let it [Bbm7]be, let it [Dbma7/Ab]be, let it [Gb2]be, let it [Db]be
[Gbma7]Whisper words of [Absus]wisdom, let it [Gb]be[Db/F][Ebm][Db]`;

const song = new ChordsOverWordsParser().parse(chordOverWords);
const actualChordPro = new ChordProFormatter().format(song);
Expand Down
8 changes: 4 additions & 4 deletions test/integration/transpose_song.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe('transposing a song', () => {

const changedSheet = heredoc`
{key: D}
Let it [Bm]be, let it [D/A]be, let it [Gsus2]be, let it [D]be`;
Let it [Bm]be, let it [D/A]be, let it [G2]be, let it [D]be`;

const song = new ChordProParser().parse(chordpro);
const updatedSong = song.transpose(2);
Expand Down Expand Up @@ -43,7 +43,7 @@ describe('transposing a song', () => {

const changedSheet = heredoc`
{key: C#}
Let it [A#m]be, let it [C#/G#]be, let it [F#sus2]be, let it [C#]be`;
Let it [A#m]be, let it [C#/G#]be, let it [F#2]be, let it [C#]be`;

const song = new ChordProParser().parse(chordpro);
const updatedSong = song.transposeUp();
Expand Down Expand Up @@ -75,7 +75,7 @@ describe('transposing a song', () => {

const changedSheet = heredoc`
{key: Db}
Let it [Bbm]be, let it [Db/Ab]be, let it [Gbsus2]be, let it [Db]be`;
Let it [Bbm]be, let it [Db/Ab]be, let it [Gb2]be, let it [Db]be`;

const song = new ChordProParser().parse(chordpro);
const updatedSong = song.transposeDown();
Expand Down Expand Up @@ -124,7 +124,7 @@ describe('transposing a song', () => {

const changedSheet = heredoc`
{key: C}
[C]Something in the way she [Cmaj7]moves
[C]Something in the way she [Cma7]moves
{key: A}
[A]You're asking m[C#m/G#]e will my love [F#m7]grow [A/E]`;

Expand Down

0 comments on commit 0188f3c

Please sign in to comment.