manastech / middleman-search

LunrJS-based search for Middleman
MIT License
58 stars 31 forks source link

Replaced therubyracer with ExecJS #24

Open daniel-rikowski opened 7 years ago

matiasgarciaisaia commented 7 years ago

CC: @eemi

eemi commented 7 years ago

If it helps, we can test it tomorrow in our Windows 7 machines.

matiasgarciaisaia commented 7 years ago

@daniel-rikowski thanks for the PR!

It Works on my Machine™, although it seems to be yielding different results - which, on first thought, I think it shouldn't. I'll come back later when I'm able to test it a little bit further.

As always, any comments will be appreciated 👍

daniel-rikowski commented 7 years ago

I noticed that, too. Different JS engines produce different results, judging from a superficial glance in the editor.

I blamed it on internal differences in the JS interpreters, like different hash algorithms, string collations or other unspecified implementation details. I did not thoroughly check if the results were fundamentally different (like missing keywords and such)

heathd commented 7 years ago

I've tested this branch locally. It fixes issues I was having with middleman hanging when running in dev mode (seems similar to https://github.com/middleman/middleman/issues/1367).

I also generated the index with this branch and with middleman-search 0.10.0 and got the same output.

Would be great if this could be merged.

Jeeppler commented 6 years ago

Please merge this PR. I have several issues installing therubyrace it would be really nice to be able to use Nodejs (through) execjs. Execjs and NodeJS are pre-packaged with most Linux distros.