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

Add process.path and process.args #29

Closed
wants to merge 1 commit into from
Closed

Add process.path and process.args #29

wants to merge 1 commit into from

Conversation

yaodingyd
Copy link
Contributor

fix #7

@sindresorhus
Copy link
Owner

Can you confirm this follows the description in #7:

I looked into this more, and I might have a solution to get the arguments without the full path. It seems ps -o pid -o comm returns the full path without arguments and ps -o pid -o command returns with arguments. So we could get both columns and just diff them to get the arguments. I think this should be added as a new property in ps-list. So we could add an args property with just the arguments and a path property with the full path to the binary without arguments, just for completeness.

@sindresorhus
Copy link
Owner

This is missing docs and TS definitions.

@yaodingyd
Copy link
Contributor Author

Yes, my implementation follows the suggestion:

  1. use comm for path is pretty straightforward
  2. args is alias for command. I use value.args.split(value.comm)[1].trim() to achieve diff between comm and args results.

@sindresorhus
Copy link
Owner

// @stroncium

@@ -51,6 +51,8 @@ const main = async (options = {}) => {
pid: Number.parseInt(key, 10),
name: path.basename(value.comm),
cmd: value.args,
path: value.comm,
args: value.args.split(value.comm)[1].trim(),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer using startsWith and slice here. The current way will fail if the args contain an argument that is the same as comm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get you. 1) comm only returns the path name; 2) if they are the same, that means there is no argument and it returns empty string

@sindresorhus sindresorhus changed the title Add process.path and process.args Add process.path and process.args Oct 3, 2019
@stroncium
Copy link
Contributor

stroncium commented Oct 6, 2019

It doesn't have conflicts with master in terms of code text, but does have them in terms of functionality atm.

@yaodingyd
Copy link
Contributor Author

@stroncium Could you elaborate?

@stroncium
Copy link
Contributor

stroncium commented Oct 6, 2019

@yaodingyd 285f434

Tests fail after merge. And since the changes are only to the case of multiple ps runs(which is left in place for the moment in case parsing single ps run output fails), your code won't even be executed normally.

@yaodingyd yaodingyd closed this Oct 31, 2019
@yaodingyd yaodingyd deleted the comm branch October 31, 2019 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new properties for the path and arguments separately
3 participants