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

Added support for Toybox to remote process picker #11175

Conversation

michalmaka
Copy link
Contributor

Hello and thanks for maintaining this repo!

I've come along a problem where my remote target needed to have different arguments for ps command in order to retrieve remote processes list. It was using ToyBox's ps and I added option for specifying custom command for getting processes list.

Current approach allows either using ps with different arguments or even using other command, see examples:

      "pipeTransport": {
        ..
        "customRemoteProcessCommand": "ps -A"
      },
      "pipeTransport": {
        ..
        "customRemoteProcessCommand": "uname && /home/root/get_pid.sh"
      },

One needs to have in mind that the custom command needs to return output in the same format as currently used commands do

@michalmaka
Copy link
Contributor Author

@microsoft-github-policy-service agree company="Software Mansion"

@Colengms Colengms requested a review from WardenGnaw July 11, 2023 23:48
@WardenGnaw
Copy link
Member

One needs to have in mind that the custom command needs to return output in the same format as currently used commands do

This would lead to more issues for users tyring to get their command to match what PsProcessParser expects and make it difficult for the extension authors decide to add more columns to the process listing. If more columns were added or reorganized, this would also lead to breaking user's launch.jsons customRemoteProcessCommand.

We would reccomend writing a vscode input variable instead, as users can run their own vscode command and parse their own ouput to the value they want for the launch.json.

https://code.visualstudio.com/docs/editor/variables-reference#_input-variables

E.g. launch.json

{
  "configurations": [
    {
      "type": "cppdbg",
      "request": "attach",
      "name": "Run Program",
      "program": "${workspaceFolder}/a,out",
      "processId": "${input:pickProcess}"
    }
  ],
  "inputs": [
    {
      "id": "pickProcess",
      "type": "command",
      "command": "extension.my.cysto.testPicker"
    }
  ]
}

@michalmaka
Copy link
Contributor Author

what about forcing such custom command to return data as CSV? Having this implemented for custom command will avoid this issue

@WardenGnaw
Copy link
Member

what about forcing such custom command to return data as CSV? Having this implemented for custom command will avoid this issue

That sounds good, I think there also need to be an associated field indicating the format of the output.

E.g.
$PID,$EXE,$CMDLINE
and use a custom parser find those token and create the correct Process object.

Another question is that is there a reason why ToyBox's ps deviates from the standard usage? Is there a good way to detect that the ps being used is ToyBoy's version?

@michalmaka michalmaka force-pushed the feature/add-custom-command-for-remote-process-list branch from bd46a26 to 2053fc9 Compare July 25, 2023 10:28
@michalmaka michalmaka changed the title Added option for custom command for getting remote processes list Added support for Toybox to remote process picker Jul 25, 2023
@michalmaka
Copy link
Contributor Author

@WardenGnaw I've changed implementation to just support Toybox detection and proper command for getting remote process list.

For some reason Toybox's ps works in a bit different way.

@sean-mcmanus sean-mcmanus merged commit a7870fc into microsoft:main Jul 26, 2023
4 checks passed
@michalmaka michalmaka deleted the feature/add-custom-command-for-remote-process-list branch August 2, 2023 11:50
bobbrow added a commit that referenced this pull request Aug 16, 2023
bobbrow added a commit that referenced this pull request Aug 16, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants