Drop env's -S flag. (#54)

[yeldiRium]

I want to offer an implementation that achieves what I have detailled in #54.

I've added tests according to the test setup as I have found it. Please tell me if I am missing quality control somewhere.

References

• [FEATURE] drop env's -S flag #54

[nathanlepori]

I'm having the issue described in #54 as well. I don't see any side effects or downsides, any chance to get this merged?

[huan]

LGTM

This is the solution that cost me 2+ hours (huan/sidecar#19) yesterday, thank you very much for the fix and I hope it can be available in the most recent NPM versions published soon.

[yeldiRium]

I'll ping the most active contributors in hopes that they have maintainer rights and take a look at this.
@isaacs @ForbesLindesay

[isaacs]

Looks fine to me. We should probably ignore any --prefixed arguments to env in the shebang, tbh.

(Note: I'm no longer a committer on this project. cc: @darcyclarke to have someone on the npm cli team take a look.)

[huan]

I hope this can be fixed in 2022! :)

[darcyclarke]

@ruyadorno / @nlf can either of you take a look at this if/when you may get some time over the next week or so.

[huan]

ping @ruyadorno @nlf

[edemaine]

+1 for this being merged. See my comment #54 (comment) for why the current state makes cross-platform "bin" impossible.

We should probably ignore any --prefixed arguments to env in the shebang, tbh.

This is less clear to me. For example, -u should unset environment variables. Supporting these correctly would require more work, so it seems better to leave them in and crash, to indicate lack of support.

Luckily, the uses of -S require that it must be the very first argument, so this PR supports what we need for crossplatform scripts.

A small comment: I think (?:\/usr\/bin\/env)?\s* should be (?:\/usr\/bin\/env)?\s+ to prevent /usr/bin/envfoo being interpreted as an env shebang line. This is a bug in the current regexp, not directly relevant to this PR, though.

@yeldiRium Could you maybe resolve the conflicts to increase chance of merging?

[yeldiRium]

Hi @edemaine,

thanks for the feedback. I updated the branch.

I didn't follow the project in the meantime, so I'm not sure if my changes are still valid. What stands out is that there are ESLint errors that occur outside of my changes.

[nlf]

this pull request got a bit stale (our fault! sorry about that!) so i went ahead and rebuilt it as #96 and am closing this one

thank you! for the contribution, it's hugely appreciated and i'm really sorry this didn't go out much sooner!