phetsims / chipper

Tools for developing and building PhET interactive simulations.
MIT License
11 stars 14 forks source link

Cleaner parse in test-sims #496

Closed samreid closed 7 years ago

samreid commented 7 years ago

in https://github.com/phetsims/chipper/issues/490#issuecomment-248969312 @pixelzoom said:

Regarding test-sims.js... This would be cleaner, and doesn't require a function:

// split string into an array of sim names, ignoring blank lines
simNames = simListText.trim().replace( /^\s*[\r\n]/gm, '' ).split( '\n' );

E.g. in console:

> var simListText = "a\nb\n\nc\nd\n\n";
undefined
> simListText.trim().replace( /^\s*[\r\n]/gm, '' ).split( '\n' );
["a", "b", "c", "d"]

Either way is fine with me, but since I closed #490 I thought I'd track this separately. @pixelzoom go for it if you wish.

pixelzoom commented 7 years ago

Done.

I first verified that the algorithm yields identical results by temporarily adding this:

var betterNames = simListText.trim().replace( /^\s*[\r\n]/gm, '' ).split( '\n' );
console.log( JSON.stringify( simNames ) === JSON.stringify( betterNames ) );

After making the change, I tested by running test-server.html.

Everything looked OK, so closing. @samreid take a peek if you wish.

samreid commented 7 years ago

I tested the regex in the console, seems OK.

pixelzoom commented 7 years ago

@jonathanolson reported on Skype:

[9/28/16, 1:34:19 PM] Jonathan Olson: Has anyone on Windows run the test-server.js (the part that builds the sims) recently? [9/28/16, 1:35:57 PM] Jonathan Olson: It's trying to build acid-base-solutions area-builder area-model-multiplication arithmetic atomic-interactions balancing-act balancing-chemical-equations balloons-and-static-electricity beaker beers-law-lab bending-light blast build-a-molecule build-an-atom calculus-grapher capacitor-lab-basics chains charges-and-fields circuit-construction-kit-black-box-study color-vision concentration curve-fitting energy-forms-and-changes energy-skate-park-basics estimation example-sim expression-exchange faradays-law fluid-pressure-and-flow forces-and-motion-basics fraction-comparison fraction-matcher friction function-builder gene-expression-essentials graphing-lines graphing-quadratics gravity-and-orbits gravity-force-lab gravity-force-lab-basics griddle hookes-law isotopes-and-atomic-mass john-travoltage joist least-squares-regression making-tens masses-and-springs models-of-the-hydrogen-atom molarity molecule-polarity molecule-shapes molecule-shapes-basics molecules-and-light neuron ohms-law pendulum-lab ph-scale ph-scale-basics plinko-probability projectile-motion proportion-playground protein-synthesis reactants-products-and-leftovers resistance-in-a-wire rutherford-scattering scenery-phet seasons simula-rasa states-of-matter states-of-matter-basics sun trig-tour under-pressure unit-rates vegas vibe wave-on-a-string [9/28/16, 1:36:03 PM] Jonathan Olson: as one sim name ... [9/28/16, 1:38:33 PM] Chris Malley: Try with simNames = simListText.trim().replace( /\r/g, '' ).split( '\n' ); [9/28/16, 1:38:55 PM] Chris Malley: StackOverflow recommended the replacement that's in master. [9/28/16, 1:38:58 PM] Jonathan Olson: working (with what you just mentioned above)

Fixed in above commit.