search

ndparker / rjsmin

Fast javascript minifier for Python
http://opensource.perlig.de/rjsmin/
Apache License 2.0
60 stars 15 forks source link

Expose python_only=True to module exports #11

Closed allada closed 5 years ago

allada commented 7 years ago

We had an external contributor request that we force users to use "python_only=True" to ensure we don't accidentally grab an older C version in the system.

See: https://codereview.chromium.org/2800063003

We (chrome devtools) needs to have a version of rjsmin at least to a specific version in order to work properly, but we don't have a way to use the exported functions available to tell it to always use the python version that is bundled with our own code.

Can we maybe add something like:

__all__ = ['jsmin', 'jsmin_python']
...
jsmin_python = _make_jsmin(python_only=True)

This would allow us to ensure we don't grab a possibly bundled version of rjsmin.

Thanks!

ndparker commented 7 years ago

Hi there,

thanks for the report.

One question in front: I guess, you're not using the commandline interface? (because there would be the -p switch).

Regarding the actual issue: I think, the jsmin_python idea is fine. Additionally I probably should put the version into the C extension and fall back to python if it's not the same.

Let's hope I find some cycles soon.

allada commented 7 years ago

I am not 100% sure why we do not use the python interface, maybe because we don't always have access to terminal? In any case we use the python bindings (this portion of our build system is in python anyway).

Maybe @wwwillchen might know more on why we don't use command line bindings.

aslushnikov commented 7 years ago

@ndparker we have python build script which uses rjsmin as one of the steps - thus we're importing it rather then invoking via CLI.

ndparker commented 5 years ago

This has finally been fixed in Release 1.1.0 - kind of. I did the version match between python and C. You'll automatically get the python version if the C version doesn't match.