Fix documentation of process.argv[0] by tarungarg546 · Pull Request #7449 · nodejs/node · GitHub

Checklist

documentation is changed or added

Affected core subsystem(s)

doc

Description of change

This PR is in reference for this issue.

Modified docs to reflect original value of process.argv[0]

Could you format the commit message according to https://github.com/nodejs/node/blob/master/CONTRIBUTING.md#step-3-commit.

The current documentation states that if run something like node app.js then in our process.argv array first elements is node, but actually its process.execPath not node as documentation currently suggests This commit fixes this documentation bug. Fixes : nodejs#7434 PR-URL: nodejs#7449

While we're in here, we may as well change "name of the JavaScript file" to something like "path to the JavaScript file."

Seems fair enough?
What say @cjihrig

I agree with @mscdex. Also, this line should be wrapped at 80 characters.

Hm. Is it a bug then that running a file named test.js using node test will lead to process.argv[1] === '/path/to/test', not /path/to/test.js?

hmm.. that's an interesting question... but I think that's ok. While there is the potential for conflicts because of the missing .js, it's not likely to have a serious impact. I guess that someone could come along and create a separate file named test after the app was started that would lead to issues but that seems like a relatively safe edge case.

Done @cjihrig

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

I think the part about the second element should be a separate sentence:

The second element will be the path to the JavaScript file being executed.

Done

I think the part about the second element should be a separate sentence:

It looks like it's still one sentence, just with 'the' removed in two places?

Oh sorry forgot to push changes.

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

No need to split all of these across lines. Just wrap them at 80 characters. Also, the sentence should end with a period, not a comma.

done.

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

a -> an

These changes are just some grammatical changes like using `an` not `a` and space after period etc.

LGTM

So sad... we'll have to change our code...

@Dmitry-Me It might help if you could describe your use case (but probably better in the original issue, as long as it doesn’t concern this documentation change itself).

@addaleax We host a piece of sample code for our users. Because it's JavaScript some of them try to run it inside web browser, which of course doesn't work. So we need a check that effectively "code is being run under NodeJS". Earlier we could just check that argv[0]=='node' but now we'll have to extract the executable name, perhaps remove the extension (somepath/node.exe on Windows and I guess it's just somepath/node on Linux) and all that stuff.

@Dmitry-Me Checking argv[0] == 'node' wouldn’t be very reliable anyway, e.g. it wouldn’t stop anyone from giving using the full node path as the first argument. If you really need some kind of check, you could test for e.g. the presence of the global process object.

@addaleax Would just checking for presence of process be enough?

@Dmitry-Me For the scenario you described, i.e. users trying to run the file in a browser with nothing else going on, yes.

Google will gladly lead you to a thousand different ways to check whether code is being run under node or not (including testing process, module or require).

EDIT: Again, if there are more follow-up questions, these probably are better asked at the original issue (nodejs/help might be a better place, too).

process.execPath is not a function, so I’d drop the parentheses?

Done 👍

LGTM with a nit.

LGTM

What's next?

Landed in 475dc43, thank for the contribution!

The current documentation states that if run something like `node app.js` then in our process.argv array first elements is `node`, but actually it's `process.execPath` not `node` as documentation currently suggests. Fixes: #7434 PR-URL: #7449 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

For historical and other reasons, node overwrites `argv[0]` on startup. See - 2c6f79c, - nodejs#7434 - nodejs#7449 - nodejs#7696 For cases where it may be useful, save the original value of `argv[0]` in `process.argv0`

For historical and other reasons, node overwrites `argv[0]` on startup. See - 2c6f79c, - #7434 - #7449 - #7696 For cases where it may be useful, save the original value of `argv[0]` in `process.argv0` PR-URL: #7696 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>

Updated ReadMe for nodejs#7434

7e8ab82

nodejs-github-bot added doc Issues and PRs related to the documentations. process Issues and PRs related to the process subsystem. labels Jun 27, 2016

tarungarg546 mentioned this pull request Jun 27, 2016

process.argv[0] contains full node.exe path on Windows, docs say it must be just "node" #7434

Closed

mscdex reviewed Jun 27, 2016
View reviewed changes

Fishrock123 changed the title Updated ReadMe for #7434 Fix documentation of process.argv[0] Jun 28, 2016

Changes as suggested by @mscdex & @cjihrig

e620fa0

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

cjihrig reviewed Jun 29, 2016
View reviewed changes

Changes as suggested by @mscdex & @cjihrig

7b0096e

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

cjihrig reviewed Jun 29, 2016
View reviewed changes

Changes as suggested by @mscdex & @cjihrig

4cd4026

With this commit everyline of process documentation is wrapped in 80 characters and there are some changes for documention of process.arv[0] and process.argv[1]

cjihrig reviewed Jun 29, 2016
View reviewed changes

Minor grammatical changes in doc for process api

9afa7fd

These changes are just some grammatical changes like using `an` not `a` and space after period etc.

addaleax reviewed Jul 1, 2016
View reviewed changes

Drop paranthesis around process.execPath

69456e4

addaleax closed this Jul 4, 2016

Fishrock123 mentioned this pull request Jul 5, 2016

Propose v6.3.0 (v2) #7550

Merged

MylesBorins mentioned this pull request Jul 11, 2016

Audit commits not found on v4.x-staging #7661

Closed

MylesBorins added the dont-land-on-v4.x label Jul 12, 2016

addaleax mentioned this pull request Jul 13, 2016

lib: allow custom names for spawned processes #7696

Closed

4 tasks

               The `process.argv` property returns a array containing the command line
               arguments passed when the Node.js process was launched. The first element will
-              be 'node', the second element will be the name of the JavaScript file. The
+              be  [`process.execPath()`], the second element will be the name of the JavaScript file. The

               arguments passed when the Node.js process was launched. The first element will
-              be 'node', the second element will be the name of the JavaScript file. The
-              remaining elements will be any additional command line arguments.
+              be [`process.execPath()`], second element will be the path to JavaScript file.

-              be 'node', the second element will be the name of the JavaScript file. The
-              remaining elements will be any additional command line arguments.
+              arguments passed when the Node.js process was launched.
+              The first element will be [`process.execPath()`],

-              arguments passed when the Node.js process was launched. The first element will
-              be 'node', the second element will be the name of the JavaScript file. The
-              remaining elements will be any additional command line arguments.
+              The `process.argv` property returns a array containing the command line

-              remaining elements will be any additional command line arguments.
+              The `process.argv` property returns an array containing the command line
+              arguments passed when the Node.js process was launched. The first element will
+              be [`process.execPath()`]. The second element will be the path to the

Fix documentation of process.argv[0] by tarungarg546 · Pull Request #7449 · nodejs/node · GitHub

Uh oh!

Uh oh!

Fix documentation of process.argv[0] - #7449

Fix documentation of process.argv[0]#7449tarungarg546 wants to merge 7 commits intonodejs:masternodejs/node:masterfrom tarungarg546:patch-1tarungarg546/node:patch-1Copy head branch name to clipboard

Conversation

tarungarg546 commented Jun 27, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

cjihrig commented Jun 27, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cjihrig commented Jun 29, 2016

Uh oh!

mscdex commented Jun 29, 2016

Uh oh!

Dmitry-Me commented Jul 1, 2016

Uh oh!

addaleax commented Jul 1, 2016

Uh oh!

Dmitry-Me commented Jul 1, 2016

Uh oh!

addaleax commented Jul 1, 2016

Uh oh!

Dmitry-Me commented Jul 1, 2016

Uh oh!

addaleax commented Jul 1, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

addaleax commented Jul 1, 2016

Uh oh!

jasnell commented Jul 1, 2016

Uh oh!

tarungarg546 commented Jul 2, 2016

Uh oh!

addaleax commented Jul 4, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Fix documentation of process.argv[0]#7449
tarungarg546 wants to merge 7 commits into
nodejs:masterfrom
tarungarg546:patch-1

addaleax commented Jul 1, 2016 •
edited
Loading