BenTalagan / glaemscribe

Glaemscribe, the tolkienian languages/writings transcription engine.
https://glaemscrafu.jrrvf.com/english/glaemscribe.html
Other
43 stars 8 forks source link

Fixing js for strict mode #17

Closed TroyAlford closed 6 years ago

TroyAlford commented 6 years ago

When attempting to import these JavaScript files webpack as modules, a number of errors crop up, because webpack forces strict mode.

This PR basically just adds var in front of the otherwise undefined variables. In addition, it adds a Gemfile and Gemfile.lock, to make it a little easier for contributors to get started contributing (bundle install).

Finally, it updates the delivery targets, and bumps the version number to 1.1.15.

TroyAlford commented 6 years ago

The work here is in support of a reactjs port of this lib. :) https://github.com/TroyAlford/react-glaemscribe

BenTalagan commented 6 years ago

Hi Troy, this is pretty cool, and I wish you good luck and work on that project!

Thanks a lot for this PR, I'll look at a way to enable strict mode at "compilation" to avoid that kind of bad surprises because it is really easy to forget to add missing "var" keywords in front of locals.

I'll merge your PR in the next few days and integrate these fixes in the next release, where I have a few others features and other fixes pending (but I'm a bit late on my schedule). Hopefully it'll be done for Tolkien's reading day (March 25th), if you can live on your own branch for a few days :)

TroyAlford commented 6 years ago

Oh awesome. :) Yeah - I'm totally happy to continue forth on my own branch at the moment.

FWIW - the way I located all of the spots where there was a missing var was to enable eslint. I didn't add that to the PR - because it also involved adding a package.json, node_modules, and a bit more. I would be happy to add this stuff in, though, if you think it would be useful? It would definitely help other collaborators get started if they wanted to contribute.

TroyAlford commented 6 years ago

BTW - I published my NPM package today. :) Here's a link, in case you'd like to check it out. It's not full-featured yet - but it's a good start. https://troyalford.github.io/react-glaemscribe

BenTalagan commented 6 years ago

Hi Troy, these fixes from your PR are incorporated to the next flavour of glaemscribe (1.2.0) which should be released in a few weeks (not yet ready, a lot of things have been done, and a small number of things are still to do) ; since I have done things in a slightly different way, I will not merge directly your PR (but it's like I had done it). Thanks a lot for having pointed these issues out!

TroyAlford commented 6 years ago

Thank you!

On Sat, Sep 22, 2018, 7:07 AM Ben Talagan Babut notifications@github.com wrote:

Closed #17 https://github.com/BenTalagan/glaemscribe/pull/17.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/BenTalagan/glaemscribe/pull/17#event-1861611970, or mute the thread https://github.com/notifications/unsubscribe-auth/ALsdfDnnWhaU6Y7_iloyYFltH7fE8PuZks5udkQigaJpZM4SlqAs .