Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process.argv behaves differently to nodejs #1752

Closed
chrislloyd opened this issue Oct 6, 2011 · 17 comments
Closed

process.argv behaves differently to nodejs #1752

chrislloyd opened this issue Oct 6, 2011 · 17 comments
Assignees
Labels

Comments

@chrislloyd
Copy link
Contributor

In a file (both valid coffee script and vanilla JS):

console.log(process.argv)

then call with:

$ node argv.js -myargs
[ 'node', '/Users/Chris/Desktop/argv.js', '-myargs' ]

and

$ coffee argv.js -myargs
[ 'coffee',
  '/Users/Chris/Desktop/argv.js',
  '-m',
  '-y',
  '-a',
  '-r',
  '-g',
  '-s' ]

CoffeScript still interprets flags after the script name.

@yuchi
Copy link

yuchi commented Oct 6, 2011

I'm not sure, but I think it's a feature.

Update: as expected, this is a feature. - args are flags, and you should be able to search for them directly as -m,-y,-a,-r,-g,-s. The -- args are long name arguments, so these are not broke into flags. just try with

    > coffee test.js --myargs

@michaelficarra
Copy link
Collaborator

A quick look yielded some results. normalizeArguments is changing the args list passed to OptionParser::parse so that it can iterate over the single-char options separately when given in that compressed style. When it sees a non-option argument (like the filename), it returns the remaining arguments... but they've already been split apart. Doesn't seem like an easy fix.

edit: node doesn't have to parse the compressed (POSIX?) style arguments, since they don't accept them. Lucky them.

edit: thought of a solution: keep the original argument list and do an indexOf to find the non-option argument in the original list and slice from there. assigning it to me.

@ghost ghost assigned michaelficarra Oct 6, 2011
@michaelficarra
Copy link
Collaborator

@yuchi: coffee should not be normalising the arguments passed to scripts; only the ones it is handling. This is a bug.

@yuchi
Copy link

yuchi commented Oct 6, 2011

Ops... looks like the correct behaviour to me. Thanks @michaelficarra

@michaelficarra
Copy link
Collaborator

@chrislloyd: fixed by e686e3f

@chrislloyd
Copy link
Contributor Author

Bam! That's service for you. Thanks!

@felixrabe
Copy link

I'm afraid this issue is not resolved.

$ cat argv.js
console.log(process.argv)

$ node -v
v0.10.9

$ coffee -v
CoffeeScript version 1.6.2

$ node argv.js -myargs
[ 'node',
  '/path/to/argv.js',
  '-myargs' ]

$ coffee argv.js -myargs
[ 'coffee',
  '/path/to/argv.js',
  '-m',
  '-y',
  '-a',
  '-r',
  '-g',
  '-s' ]

@felixrabe
Copy link

I just verified that e686e3f really did fix this issue. There must have been a regression in the meantime.

$ pwd
/path/to/coffee-script

$ echo 'console.log(process.argv)' > ./argv.js

$ git checkout -b test-before e686e3f6e9b47949f12567d04dd25eae63ed3ce5~
Switched to a new branch 'test-before'

$ ./bin/coffee argv.js -myargs
path.exists is now called `fs.exists`.
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-m',
  '-y',
  '-a',
  '-r',
  '-g',
  '-s' ]

$ git checkout -b test-after e686e3f6e9b47949f12567d04dd25eae63ed3ce5
Switched to a new branch 'test-after'

$ ./bin/coffee argv.js -myargs
path.exists is now called `fs.exists`.
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-myargs' ]

Please reopen this issue. I will comment again with the commit that re-introduced the bug and then leave it to you (unless a fix is obvious to me).

@vendethiel vendethiel reopened this Jun 1, 2013
@felixrabe
Copy link

The bug was re-introduced in c0dac45: (and never "re-fixed" in the meantime)

c0dac45 OptionParser and related tests needed a cleanup

Bisection:

$ git log --pretty=oneline --abbrev-commit test-after..master src/optparse.coffee
7be996c code cleanup
c0dac45 OptionParser and related tests needed a cleanup
e2a205a making use of slicing syntax
fc0a169 fixes #1910: loop index should be mutable within a loop iteration and immutable between loop iterations
054fe34 fixes #1754: support filenames starting with `-` by using `--` arg

$ git checkout -B test 054fe34
Switched to a new branch 'test'

$ ./bin/coffee argv.js -myargs
path.exists is now called `fs.exists`.
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-myargs' ]

$ git checkout -B test fc0a169
Reset branch 'test'

$ ./bin/coffee argv.js -myargs
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-myargs' ]

$ git checkout -B test e2a205a
Reset branch 'test'

$ ./bin/coffee argv.js -myargs
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-myargs' ]

$ git checkout -B test c0dac45
Reset branch 'test'

$ ./bin/coffee argv.js -myargs
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-m',
  '-y',
  '-a',
  '-r',
  '-g',
  '-s' ]

$ git checkout -B test 7be996c
Reset branch 'test'

$ ./bin/coffee argv.js -myargs
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-m',
  '-y',
  '-a',
  '-r',
  '-g',
  '-s' ]

@vendethiel
Copy link
Collaborator

Thanks for finding c0dac45 :).

@felixrabe
Copy link

Well, thanks for fixing it :)

@xixixao
Copy link
Contributor

xixixao commented Dec 17, 2013

The desired behavior is achieved by

$ ./bin/coffee argv.js -- -myargs
[ 'coffee',
  '/path/to/coffee-script/argv.js',
  '-myargs']

Close?

@felixrabe
Copy link

Ok, so coffee --help gives me:

Usage: coffee [options] path/to/script.coffee -- [args]

which explicitly states the -- to be necessary, and I successfully use the following shebang line as well:

#!/usr/bin/env coffee --

As this is documented to work this way, I would accept this issue to get closed.

@slezica
Copy link

slezica commented Aug 18, 2014

@felixrabe I don't know. It is documented, yes; but if you're not familiar with the -- convention (and a Unix beginner certainly won't be) this is completely unintuitive, reinforced by the fact that people coming from javascript won't expect it since bare node.js behaves differently.

I think the reasonable behavior would be to pass all arguments after the script name untouched, at least if not compiling.

@cosmicexplorer
Copy link
Contributor

Unless I'm very much mistaken, not all versions of env work the same, or I'm calling it incorrectly. Prefacing a coffeescript file with #!/usr/bin/env coffee -- does not work (this is on coffeescript 1.9.1), and this issue makes developing an npm module much more frustrating. The input:

#!/usr/bin/env coffee --

console.log process.argv

fails when run with the error /usr/bin/env: coffee --: No such file or directory. If the -- is removed, of course, then process.argv "unpacks" packed arguments, turning -ABCD into -A -B -C -D, which screws over the argument processing for the application I'm trying to write (I'm working around it, but it's frustrating). This doesn't need to be a problem; the command processor just needs to not modify the version of process.argv it passes into the forkNode function. I'll make a pull request if it has any chance of getting approved, it'll take five minutes.

@cosmicexplorer
Copy link
Contributor

So I did some reading and found this thread which states that some operating systems parse the env line differently. I tested on a mac and found that #!/usr/bin/env coffee -- does work there, as it is parsed by the operating system as a call to /usr/bin/env coffee -- ("coffee" and "--" are parsed as separate arguments). However, on every Linux device I've tested it on, it fails, parsing the shebang line as a call to /usr/bin/env "coffee --"; obviously, there is no executable on my path named "coffee --", so the call to /usr/bin/env fails.

The best way to deal with this that I can think of is (if there is no -- argument given) to just check if the first argument to the interpreter is a coffeescript file, and then leaving all options to the right of that alone and passing them directly to the node process. Again, I'd do it, but this also might break some existing scripts that rely on being able to put arguments to coffeescript after the input script name (although running coffee -h shows that the correct way to run it is to provide all arguments to the coffeescript compiler before the input script argument, but obviously the -- argument was introduced so that behavior was not strictly enforced), which is not fun. The probability of that is likely much less than the probability of someone trying to run a coffeescript file using a shebang line on a Linux system, though, so I think it's an appropriate tradeoff.

cosmicexplorer pushed a commit to cosmicexplorer/coffeescript that referenced this issue Apr 16, 2015
Pass arguments to executable script unchanged if using "#!/usr/bin/env
coffee". (Previously, "./test.coffee -abck" would be turned into "-a -b -c -k",
for example.)

Fixes jashkenas#1752.
@jashkenas jashkenas removed this from the as soon as a patch is available milestone Jun 2, 2015
@GeoffreyBooth
Copy link
Collaborator

As commented above, this is documented in coffee --help, and behaving as designed. The coffee binary is intended primarily for compiling and debugging, not as a replacement for node.

GeoffreyBooth pushed a commit that referenced this issue Jul 19, 2017
* Add #! support for executable scripts on Linux.

Pass arguments to executable script unchanged if using "#!/usr/bin/env
coffee". (Previously, "./test.coffee -abck" would be turned into "-a -b -c -k",
for example.)

Fixes #1752.

* refactor option parsing

clean up parsing code and in the process fix oustanding bug where coffeescript
modified arguments meant for an executable script

* address comments

* intermediate save

* add note saying where OptionParser is used in coffee command

* add some more work

* fix flatten functions

* refactor tests

* make argument processing less confusing

* add basic test

* remove unused file

* compilation now hangs

* remove unnecessary changes

* add tests!!!

* add/fix some tests

* clarify a test

* fix helpers

* fix opt parsing

* fix infinite loop

* make rule building easier to read

* add tests for flag overlap

* revamp argument parsing again and add more thorough testing

* add tests, comment, clean unused method

* address review comments

* add test for direct invocation of shebang scripts

* move shebang parsing test to separate file and check for browser

* remove TODO

* example backwards compatible warnings

* add correct tests for warning 1

* add tests for warnings

* commit output js libs and update docs

* respond to review comments

also add tests for help text

* respond to review comments

* fix example output

* Rewrite argument parsing documentation to be more concise; add it to sidebar and body; add new output

* Don’t mention deprecated syntax; clean up variable names
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants