Review generate HTML files

[Denz1994]

Related to #872 for reviewing generateDevelopmentHTML.js and generateTestHTML.js:

generateDevelopmentHTML.js

• Top-level documentation reads "... using the current brand's splash and preloads". However const brands = 'phet' is hard-coded (line 43). Should the top-level doc be specific to phet brand because it is hard-coded?

• Documentation in generateTestHTML.options.stylesheets reads : // Note the preceding whitespace which makes the formatting match IDEA formatting. Should bodystyle in generateDevelopmentHTML.js include documentation as to why the white space is "essential"? Is it for the same reason?

• Organization: I would move the indentLines() declaration above the preloads declaration so the helper functions are grouped together

• Within stringifyArray, rename arr.map().join() to stringifiedPreload. It would make the return value more readable.

• Within stringifyArray, prefix is concatenated to the end of the return value (like this [ arr.map().join() + prefix ]. Prefix in this case isn't really a prefix, it is a suffix. Consider renaming to indentation, based on it being concatenated to the beginning and end of the string in stringifyArray.

---

generateTestHTML.js

• Shouldn't this file use strict mode? use strict is missing.

• module.exports = async ( repo, options ) is missing its jsDoc.

Assigning to @jonathanolson to respond to review comments.

[jonathanolson]

Should bodystyle in generateDevelopmentHTML.js include documentation as to why the white space is "essential"? Is it for the same reason?

I don't think a ton of comments about matching indentation style are particularly necessary. What would you change to add documentation? (Feel free to add docs)

Within stringifyArray, rename arr.map().join() to stringifiedPreload. It would make the return value more readable.

To me, the inline version is much more readable. isolating that into a variable doesn't give a whole lot more information, and the inline version shows where the content will end up in the string. Thoughts?

Otherwise, the review issues should be handled, good finds!

[Denz1994]

Re: Indentation for bodystyle; I wasn't clear on whether the indentation was required in generateDevelopmentHTML.options.bodystyle for the same reason it is required for generateTestHTML.options.stylesheets ( to match IDEA formatting).

Also, the inline version of arr.map().join() looks much better.

[Denz1994]

Closing this issue since the above items have been addressed.