Fix key signature without mode, common/alla breve time (#305)

* fix(KeySignatureParse): key signature displayed correctly when mode not set in XML

mode (major/minor/etc) doesn't need to be set per MXML standard.
Musescore sometimes exports without mode set as well.

Also added an afterthought comment line from the formatting overhaul (e3fa44f)

fix #302

* fix(TimeSignatures): common time and alla breve display correctly

fix #303

* test(key/time) add tests for key without mode element, common and alla breve time symbols

requested change for PR #305

src/MusicalScore/Graphical/VexFlow/VexFlowConverter.ts

@@ -533,7 +533,10 @@ export class VexFlowConverter {
             case KeyEnum.major:
                 ret = VexFlowConverter.majorMap[key.Key];
                 break;
+            // some XMLs don't have the mode set despite having a key signature.
             case KeyEnum.none:
+                ret = VexFlowConverter.majorMap[key.Key];
+                break;
             default:
                 ret = "C";
         }

src/MusicalScore/Graphical/VexFlow/VexFlowMusicSheetCalculator.ts

@@ -100,6 +100,7 @@ export class VexFlowMusicSheetCalculator extends MusicSheetCalculator {
             log.warn("Found a measure with no voices... Continuing anyway.", mvoices);
             continue;
         }
+        // all voices that belong to one stave are collectively added to create a common context in VexFlow.
         formatter.joinVoices(voices);
     }
 

src/MusicalScore/ScoreIO/InstrumentReader.ts

@@ -711,7 +711,6 @@ export class InstrumentReader {
           keyEnum = KeyEnum.major;
           log.debug("InstrumentReader.addAbstractInstruction", errorMsg, ex);
         }
-
       }
       const keyInstruction: KeyInstruction = new KeyInstruction(undefined, key, keyEnum);
       this.abstractInstructions.push([1, keyInstruction]);
@@ -782,9 +781,6 @@ export class InstrumentReader {
           log.debug("InstrumentReader.addAbstractInstruction", errorMsg, ex);
         }
 
-        if ((num === 4 && denom === 4) || (num === 2 && denom === 2)) {
-          symbolEnum = RhythmSymbolEnum.NONE;
-        }
         this.abstractInstructions.push([1, new RhythmInstruction(
           new Fraction(num, denom, 0, false), symbolEnum
         )]);

test/MusicalScore/ScoreIO/Key_Test.ts

@@ -3,7 +3,10 @@ import { MusicSheet }             from "../../../src/MusicalScore/MusicSheet";
 import { IXmlElement }            from "../../../src/Common/FileIO/Xml";
 import { KeyInstruction }         from "../../../src/MusicalScore/VoiceData/Instructions/KeyInstruction";
 import { KeyEnum as KeyModeEnum } from "../../../src/MusicalScore/VoiceData/Instructions/KeyInstruction";
+import { VexFlowConverter } from "../../../src/MusicalScore/Graphical/VexFlow/VexFlowConverter";
 import * as chai                  from "chai";
+import { AbstractNotationInstruction } from "../../../src/MusicalScore/VoiceData/Instructions/AbstractNotationInstruction";
+import { RhythmInstruction, RhythmSymbolEnum } from "../../../src/MusicalScore/VoiceData/Instructions/RhythmInstruction";
 
 let reader: MusicSheetReader;
 let parser: DOMParser;
@@ -257,17 +260,63 @@ describe("MusicXML parser for element 'key'", () => {
   });
 });
 
-function getMusicSheetWithKey(fifths: number = undefined, mode: string = undefined): MusicSheet {
-  const doc: Document = parser.parseFromString(getMusicXmlWithKey(fifths, mode), "text/xml");
+describe("VexFlowConverter for element 'key'", () => {
+  before((): void => {
+    reader = new MusicSheetReader();
+    parser = new DOMParser();
+  });
+
+  it("gives key signature G-major with no optional 'mode' element present", (done: MochaDone) => {
+    const keyInstruction: KeyInstruction = getMusicSheetWithKey(1, "").getFirstSourceMeasure().getKeyInstruction(0);
+    const vexflowKeySignature: string = VexFlowConverter.keySignature(keyInstruction);
+    const isGMajorOrEminor: boolean = ["G", "E"].indexOf(vexflowKeySignature.charAt(0)) !== -1;
+    chai.expect(isGMajorOrEminor).to.equal(true);
+    done();
+  });
+});
+
+// not key tests, but if we outsource this, we need to make getMusicSheetWithKey() accessible from other test files.
+describe("InstrumentReader for element 'time'", () => {
+  before((): void => {
+    reader = new MusicSheetReader();
+    parser = new DOMParser();
+  });
+
+  it("gives common time RythmSymbolEnum from xml", (done: MochaDone) => {
+    const instructions: AbstractNotationInstruction[] =
+      getMusicSheetWithKey(1, "major", "common").getFirstSourceMeasure().FirstInstructionsStaffEntries[0].Instructions;
+    for (const instruction of instructions) {
+      if (instruction instanceof RhythmInstruction) {
+        chai.expect(instruction.SymbolEnum).to.equal(RhythmSymbolEnum.COMMON);
+      }
+    }
+    done();
+  });
+
+  it("gives alla breve/cut time RythmSymbolEnum from xml", (done: MochaDone) => {
+    const instructions: AbstractNotationInstruction[] =
+      getMusicSheetWithKey(1, "major", "cut").getFirstSourceMeasure().FirstInstructionsStaffEntries[0].Instructions;
+    for (const instruction of instructions) {
+      if (instruction instanceof RhythmInstruction) {
+        chai.expect(instruction.SymbolEnum).to.equal(RhythmSymbolEnum.CUT);
+      }
+    }
+    done();
+  });
+});
+
+function getMusicSheetWithKey(fifths: number = undefined, mode: string = undefined, timeSymbol: string = ""): MusicSheet {
+  const doc: Document = parser.parseFromString(getMusicXmlWithKey(fifths, mode, timeSymbol), "text/xml");
   chai.expect(doc).to.not.be.undefined;
   const score: IXmlElement = new IXmlElement(doc.getElementsByTagName("score-partwise")[0]);
   chai.expect(score).to.not.be.undefined;
   return reader.createMusicSheet(score, "template.xml");
 }
 
-function getMusicXmlWithKey(fifths: number = undefined, mode: string = undefined): string {
+function getMusicXmlWithKey(fifths: number = undefined, mode: string = undefined, timeSymbol: string = ""): string {
   const modeElement: string = mode ? `<mode>${mode}</mode>` : "";
   const fifthsElement: string = fifths ? `<fifths>${fifths}</fifths>` : "";
+  const timeSymbolAttribute: string = timeSymbol ? `symbol="${timeSymbol}"` : "";
   return `<?xml version="1.0" encoding="UTF-8" standalone="no"?>
           <!DOCTYPE score-partwise PUBLIC
               "-//Recordare//DTD MusicXML 3.0 Partwise//EN"
@@ -286,7 +335,7 @@ function getMusicXmlWithKey(fifths: number = undefined, mode: string = undefined
                     ${fifthsElement}
                     ${modeElement}
                   </key>
-                  <time>
+                  <time ${timeSymbolAttribute}>
                     <beats>4</beats>
                     <beat-type>4</beat-type>
                   </time>