Repository: FHIR/sushi (ts)

Issue:
Generated jekyll comments interfere w/ IG Publisher parsing
SUSHI 0.11.2 introduced generated comments on SUSHI generated and copied files.  For xml files in pagecontent, we added Jekyll comments at the top of `.md` and `.xml` files.  While this works fine for `.md` files, the IG Publisher chokes on it for `.xml` files.

First it reports this error:
```
Error on line 1 column 1
  SXXP0003: Error reported by XML parser: Content is not allowed in prolog.
Exception generating page /Users/cmoesel/dev/fsh/simple-ig/build/temp/pages/_includes/index.xml: org.xml.sax.SAXParseException; lineNumber: 1; columnNumber: 1; Content is not allowed in prolog. (00:19.0789)
```

Then later on:
```
Jekyll has failed. Complete output from running Jekyll:   Liquid Exception: Could not locate the included file 'index.xml' in any of ["/Users/cmoesel/dev/fsh/simple-ig/build/temp/pages/_includes"]. Ensure it exists in one of those directories and, if it is a symlink, does not point outside your site source (00:23.0360)
Note: Check that Jekyll is installed correctly                                   (00:23.0360)
Publishing Content Failed: Process exited with an error: 1 (Exit value: 1)       (00:23.0361)
                                                                                 (00:23.0361)
Use -? to get command line help                                                  (00:23.0361)
                                                                                 (00:23.0361)
Stack Dump (for debugging):                                                      (00:23.0361)
org.apache.commons.exec.ExecuteException: Process exited with an error: 1 (Exit value: 1)
	at org.apache.commons.exec.DefaultExecutor.executeInternal(DefaultExecutor.java:404)
```

Thanks to Bob Milius for reporting this on Zulip: https://chat.fhir.org/#narrow/stream/215610-shorthand/topic/SUSHI.200.2E11.2E2/near/192873136

Pull request description:
Wrap Jekyll comments in <!-- -->
The IG Publisher doesn't deal well w/ bare Jekyll comments in XML -- so always wrap Jekyll comments in <!-- -->.  Also add the source filename in the comment so the final HTML doesn't just have an empty comment.

Fixes #338.

You can test by building an IG w/ an `ig-data/input/pagecontent/index.xml` file using the OLD SUSHI -- and IG Publisher will crash.  With the new SUSHI, it succeeds.

Patch:
diff --git a/src/ig/IGExporter.ts b/src/ig/IGExporter.ts
index 19f13ee..a9f4e09 100644
--- a/src/ig/IGExporter.ts
+++ b/src/ig/IGExporter.ts
@@ -241,7 +241,7 @@ export class IGExporter {
       logger.info('Copied ig-data/input/pagecontent/index.xml');
     } else {
       logger.info('Generated default index.md.');
-      const warning = warningText('{% comment %} ', ' {% endcomment %}', [
+      const warning = warningBlock('<!-- index.md {% comment %}', '{% endcomment %} -->', [
         'This index.md file was generated from the "description" in package.json. To provide',
         'your own index file, create an index.md or index.xml in the ig-data/input/pagecontent',
         'folder.',
@@ -662,7 +662,7 @@ export class IGExporter {
           'file in the ig-data folder with the values that should be merged into this generated file.',
           'See: https://build.fhir.org/ig/FHIR/ig-guidance/using-templates.html#root'
         ];
-    const iniWarning = warningText('; ', '', extra, false);
+    const iniWarning = [...warningTextArray('; ', extra), ''].join(EOL);
     outputIniContents = outputIniContents.replace('\n', `\n${iniWarning}`);
 
     // Finally, write it to disk
@@ -713,7 +713,7 @@ export class IGExporter {
     outputJSONSync(
       outputPath,
       {
-        '//': warningTextArray('', '', [
+        '//': warningTextArray('', [
           'This package-list.json was generated using default values and values inferred from',
           'package.json (name, title, canonical, description, url, version). This is only a starter',
           'file. IG authors should provide their own package-list.json file in the ig-data folder',
@@ -832,23 +832,29 @@ export class IGExporter {
       return;
     }
 
-    // It's a file.  Add the warning text if applicable, otherwise copy as-is
-    let prefix: string, postfix: string;
-    if (inputPath.endsWith('.xml') || inputPath.endsWith('.md')) {
-      // It's a file that will be processed by Jekyll, so go ahead and use Jekyll comments
-      [prefix, postfix] = ['{% comment %} ', ' {% endcomment %}'];
-    } else {
+    // If it's not xml or md, just copy it as is and we're done.
+    if (!inputPath.endsWith('.md') && !inputPath.endsWith('.xml')) {
       this.copyAsIs(inputPath, outputPath);
       return;
     }
 
+    // Otherwise, it's .md or .xml
     const extra = [
       'To change the contents of this file, edit the original source file at:',
-      inputPath.slice(inputPath.indexOf('/ig-data/') + 1)
+      inputPath.slice(inputPath.indexOf(`${path.sep}ig-data${path.sep}`) + 1)
     ];
-
+    // .xml files can't have bare jekyll comments at the start of the file, as they fail XML parsing,
+    // so we must surround the warning w/ XML comments.  To avoid the final HTML having just an empty
+    // XML comment tag, we add in the file name -- which is likely useful info in the source anyway;
+    // and for consistency, we do it for both .xml and .md
+    const warning = warningBlock(
+      `<!-- ${path.parse(outputPath).base} {% comment %}`,
+      '{% endcomment %} -->',
+      extra,
+      false
+    );
     const content = readFileSync(inputPath, 'utf8');
-    outputFileSync(outputPath, `${warningText(prefix, postfix, extra)}${content}`, 'utf8');
+    outputFileSync(outputPath, `${warning}${content}`, 'utf8');
     this.updateOutputLogForCopiedPath(outputPath, inputPath);
   }
 
@@ -924,24 +930,23 @@ type outputLogDetails = {
 };
 
 /**
- * Creates a set of comments indicating that a file is generated and should not be directly edited,
- * allowing for the comment delimiters to be passed in as well as any extra text to include.
+ * Creates a block of comments indicating that a file is generated and should not be directly edited,
+ * allowing for the block comment delimiters to be passed in as well as any extra text to include.
  *
- * @param prefix {string} - the comment prefix to use at the start of each line (e.g., <!--)
- * @param postfix {string} - the comment postfix to use at the end of each line (e.g., -->)
+ * @param blockPrefix {string} - the comment prefix to use at the start of the block (e.g., <!-- {% comment %})
+ * @param blockPostfix {string} - the comment postfix to use at the end of the block (e.g., {% endcomment %} -->)
  * @param extra {List<string>} - an array of strings, each of which is a line to include in the comment after the
  *   standard warning text
- * @param blankLineAfter {boolean} - whether or not to put a blank line after the comments
+ * @param blankLineAfter {boolean} - whether or not to put a blank line after the block of comments
  * @returns {string} representing the formatted comments
  */
-function warningText(
-  prefix: string,
-  postfix = '',
+function warningBlock(
+  blockPrefix = '',
+  blockPostfix = '',
   extra: string[] = [],
   blankLineAfter = true
 ): string {
-  const a = warningTextArray(prefix, postfix, extra);
-  a.push('');
+  const a = [blockPrefix, ...warningTextArray('', extra), blockPostfix, ''];
   if (blankLineAfter) {
     a.push('');
   }
@@ -954,19 +959,18 @@ function warningText(
  * allows for the comment delimiters to be passed in as well as any extra text to include.
  *
  * @param prefix {string} - the comment prefix to use at the start of each line (e.g., <!--)
- * @param postfix {string} - the comment postfix to use at the end of each line (e.g., -->)
  * @param extra {List<string>} - an array of strings, each of which is a line to include in the comment after the
  *   standard warning text
  */
-function warningTextArray(prefix: string, postfix = '', extra: string[] = []): string[] {
+function warningTextArray(prefix: string, extra: string[] = []): string[] {
   const msgLen = Math.max(85, ...extra.map(e => e.length));
   const a: string[] = [];
   const msg = (text = '', center = false): void => {
     const fn = center ? pad : padEnd;
-    a.push(`${prefix}* ${fn(text, msgLen)} *${postfix}`);
+    a.push(`${prefix}* ${fn(text, msgLen)} *`);
   };
 
-  a.push(`${prefix}${repeat('*', msgLen + 4)}${postfix}`);
+  a.push(`${prefix}${repeat('*', msgLen + 4)}`);
   msg('WARNING: DO NOT EDIT THIS FILE', true);
   msg();
   msg('This file is generated by SUSHI. Any edits you make to this file will be overwritten.');
@@ -974,6 +978,6 @@ function warningTextArray(prefix: string, postfix = '', extra: string[] = []): s
     msg();
     extra.forEach(m => msg(m));
   }
-  a.push(`${prefix}${repeat('*', msgLen + 4)}${postfix}`);
+  a.push(`${prefix}${repeat('*', msgLen + 4)}`);
   return a;
 }
diff --git a/src/ig/files/input/includes/menu.xml b/src/ig/files/input/includes/menu.xml
index 3907fa1..9926148 100644
--- a/src/ig/files/input/includes/menu.xml
+++ b/src/ig/files/input/includes/menu.xml
@@ -1,13 +1,14 @@
-{% comment %} ****************************************************************************************** {% endcomment %}
-{% comment %} *                             WARNING: DO NOT EDIT THIS FILE                             * {% endcomment %}
-{% comment %} *                                                                                        * {% endcomment %}
-{% comment %} * This file is generated by SUSHI. Any edits you make to this file will be overwritten.  * {% endcomment %}
-{% comment %} *                                                                                        * {% endcomment %}
-{% comment %} * This menu.xml file is a generic menu provided by SUSHI. To provide your own menu,      * {% endcomment %}
-{% comment %} * create a menu.xml file in your ig-data/input/includes folder.                          * {% endcomment %}
-{% comment %} * See: https://build.fhir.org/ig/FHIR/ig-guidance/using-templates.html#igroot-input      * {% endcomment %}
-{% comment %} ****************************************************************************************** {% endcomment %}
-
+<!-- menu.xml {% comment %}
+******************************************************************************************
+*                             WARNING: DO NOT EDIT THIS FILE                             *
+*                                                                                        *
+* This file is generated by SUSHI. Any edits you make to this file will be overwritten.  *
+*                                                                                        *
+* This menu.xml file is a generic menu provided by SUSHI. To provide your own menu,      *
+* create a menu.xml file in your ig-data/input/includes folder.                          *
+* See: https://build.fhir.org/ig/FHIR/ig-guidance/using-templates.html#igroot-input      *
+******************************************************************************************
+{% endcomment %} -->
 <ul xmlns="http://www.w3.org/1999/xhtml" class="nav navbar-nav">
   <li><a href="index.html">IG Home</a></li>
   <li><a href="toc.html">Table of Contents</a></li>


Test patch:
diff --git a/test/ig/IGExporter.test.ts b/test/ig/IGExporter.test.ts
index a536d2b..e2d2ae6 100644
--- a/test/ig/IGExporter.test.ts
+++ b/test/ig/IGExporter.test.ts
@@ -316,11 +316,10 @@ describe('IGExporter', () => {
       const indexPath = path.join(tempOut, 'input', 'pagecontent', 'index.md');
       expect(fs.existsSync(indexPath)).toBeTruthy();
       const content = fs.readFileSync(indexPath, 'utf8');
+      expect(content).toMatch(/^<!-- index.md \{% comment %\}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+This index\.md file was generated from the "description" in package.json\..+\{% endcomment %\}/
+        /^\*\s+This index\.md file was generated from the "description" in package.json\..+\*$/m
       );
       expect(content).toMatch('Provides a simple example of how FSH can be used to create an IG');
     });
@@ -329,11 +328,10 @@ describe('IGExporter', () => {
       const menuPath = path.join(tempOut, 'input', 'includes', 'menu.xml');
       expect(fs.existsSync(menuPath)).toBeTruthy();
       const content = fs.readFileSync(menuPath, 'utf8');
+      expect(content).toMatch(/^<!-- menu.xml {% comment %}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+This menu\.xml file is a generic menu provided by SUSHI\..+\{% endcomment %\}/
+        /^\*\s+This menu\.xml file is a generic menu provided by SUSHI\..+\*$/m
       );
       expect(content).toMatch('<li><a href="index.html">IG Home</a></li>');
       expect(content).toMatch('<li><a href="toc.html">Table of Contents</a></li>');
@@ -569,15 +567,12 @@ describe('IGExporter', () => {
       const indexPath = path.join(tempOut, 'input', 'pagecontent', 'index.md');
       expect(fs.existsSync(indexPath)).toBeTruthy();
       const content = fs.readFileSync(indexPath, 'utf8');
+      expect(content).toMatch(/^<!-- index.md {% comment %}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]pagecontent[\/\\]index\.md.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*$/m
       );
+      expect(content).toMatch(/^\*\s+ig-data[\/\\]input[\/\\]pagecontent[\/\\]index\.md\s+\*$/m);
       expect(content).toMatch('My special index page.');
 
       // Checks that the index.md file is added to IG definition
@@ -595,15 +590,12 @@ describe('IGExporter', () => {
       const menuPath = path.join(tempOut, 'input', 'includes', 'menu.xml');
       expect(fs.existsSync(menuPath)).toBeTruthy();
       const content = fs.readFileSync(menuPath, 'utf8');
+      expect(content).toMatch(/^<!-- menu.xml {% comment %}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]includes[\/\\]menu\.xml.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*$/m
       );
+      expect(content).toMatch(/^\*\s+ig-data[\/\\]input[\/\\]includes[\/\\]menu\.xml\s+\*$/m);
       expect(content).toMatch('<li><a href="index.html">My special menu</a></li>');
       expect(content).toMatch('<li><a href="toc.html">Customized Table of Contents</a></li>');
     });
@@ -624,15 +616,12 @@ describe('IGExporter', () => {
       const otherIncludeFilePath = path.join(tempOut, 'input', 'includes', 'other.xml');
       expect(fs.existsSync(otherIncludeFilePath)).toBeTruthy();
       const content = fs.readFileSync(otherIncludeFilePath, 'utf8');
+      expect(content).toMatch(/^<!-- other.xml {% comment %}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]includes[\/\\]other\.xml.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*$/m
       );
+      expect(content).toMatch(/^\*\s+ig-data[\/\\]input[\/\\]includes[\/\\]other\.xml\s+\*$/m);
       expect(content).toMatch('<li><a href="index.html">Some other non-menu file</a></li>');
     });
 
@@ -643,32 +632,28 @@ describe('IGExporter', () => {
       // All file contents get copied over
       const otherFilePath = path.join(pageContentPath, 'other-page.md');
       const otherContent = fs.readFileSync(otherFilePath, 'utf8');
+      expect(otherContent).toMatch(/^<!-- other-page.md \{% comment %\}$/m);
+      expect(otherContent).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(otherContent).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*$/m
       );
       expect(otherContent).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(otherContent).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]pagecontent[\/\\]other-page\.md.+\{% endcomment %\}/
+        /^\*\s+ig-data[\/\\]input[\/\\]pagecontent[\/\\]other-page\.md\s+\*$/m
       );
       expect(otherContent).toMatch('My other now-supported-page.');
       const unsupportedFilePath = path.join(pageContentPath, 'unsupported.html');
       const unsupportedContent = fs.readFileSync(unsupportedFilePath, 'utf8');
-      expect(unsupportedContent).not.toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
+      expect(unsupportedContent).not.toContain('WARNING: DO NOT EDIT THIS FILE');
       expect(unsupportedContent).toMatch('<p>An unsupported file type will copied over</p>');
       const notesFilePath = path.join(pageContentPath, 'resource-notes.md');
       const notesContent = fs.readFileSync(notesFilePath, 'utf8');
+      expect(notesContent).toMatch(/<!-- resource-notes.md/);
+      expect(notesContent).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(notesContent).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*$/m
       );
       expect(notesContent).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(notesContent).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]pagecontent[\/\\]resource-notes\.md.+\{% endcomment %\}/
+        /^\*\s+ig-data[\/\\]input[\/\\]pagecontent[\/\\]resource-notes\.md\s+\*$/m
       );
       expect(notesContent).toContain('Some resource specific notes.');
 
@@ -796,15 +781,12 @@ describe('IGExporter', () => {
       const indexPath = path.join(tempOut, 'input', 'pagecontent', 'index.xml');
       expect(fs.existsSync(indexPath)).toBeTruthy();
       const content = fs.readFileSync(indexPath, 'utf8');
+      expect(content).toMatch(/^<!-- index.xml {% comment %}$/m);
+      expect(content).toMatch(/^\*\s+WARNING: DO NOT EDIT THIS FILE\s+\*$/m);
       expect(content).toMatch(
-        /\{% comment %\}.+WARNING: DO NOT EDIT THIS FILE.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+To change the contents of this file, edit the original source file at:.+\{% endcomment %\}/
-      );
-      expect(content).toMatch(
-        /\{% comment %\}.+ig-data[\/\\]input[\/\\]pagecontent[\/\\]index\.xml.+\{% endcomment %\}/
+        /^\*\s+To change the contents of this file, edit the original source file at:\s+\*/m
       );
+      expect(content).toMatch(/^\*\s+ig-data[\/\\]input[\/\\]pagecontent[\/\\]index\.xml\s+\*/m);
       expect(content).toContain('An index file in XML');
 
       // Checks that the index.xml file is added to IG definition