search

python-eel / Eel

A little Python library for making simple Electron-like HTML/JS GUI apps
MIT License
6.44k stars 587 forks source link

Fix parsing minified code with PEG (#363) #365

Closed KyleKing closed 4 years ago

KyleKing commented 4 years ago

Resolves #363

Particularly in the React example, but anywhere where JS is minified, you can end up with a variety of static JS code to parse. Currently the regex will fail because it only grabs the string between eel.expose( and the first )

eel.expose(func_name) window.eel.expose(w,"say_hello_js") window.eel.expose(function(e){console.log(e)},"show_log_alt") (AssertionError: function()

Alternatively, this PR demonstrates how pyparsing could be used to parse all of the above cases suppressing the nested JavaScript function if found (pp.NestedExpr())

I haven't thoroughly tested these changes and only ran examples 1, 4, and 7 to quickly verify that it works for most cases. I would like to at least test with Electron and think more about what kind of edge cases could happen

KyleKing commented 4 years ago

@samuelhwilliams, I added tests and think this PR is ready for review

samuelhwilliams commented 4 years ago

Hey @KyleKing - this looks like a great upgrade to Eel's parsing stuff in principle. I'm going to drop some comments on it over the next day or two as/when I can - feel free not to start responding until I say I've finished so that it's not too bitty for you.

Sorry it's taking so long.

KyleKing commented 4 years ago

No problem, I was up this morning and that was an easy fix. Let me know if there are other improvements!

samuelhwilliams commented 4 years ago

Hey @KyleKing - I'm done reviewing now. There's a few minor comments above but it looks really good on the whole. Once you've finished updating things, could you rebase and sanitise the commits a little (i.e. probably just one commit makes sense here) - and then I'll merge.

Thanks so much for this!

KyleKing commented 4 years ago

@samuelhwilliams I made all of the changes. Thanks for the edits

I'm not sure how to squash/rebase the commits without force pushing to Github or creating a new branch and starting a new PR. Do you have the option to squash the commits below? https://github.blog/2016-04-01-squash-your-commits/

samuelhwilliams commented 4 years ago

I can squash them, that's fine, but force pushing to your branch would also presumably have been fine. Thanks for the updates!

ChrisKnott commented 4 years ago

This is a really great change @KyleKing

KyleKing commented 4 years ago

Thanks!