Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Show warning if 'program' attribute is used in 'remote' #2999

Merged
merged 3 commits into from
Mar 24, 2020
Merged

Show warning if 'program' attribute is used in 'remote' #2999

merged 3 commits into from
Mar 24, 2020

Conversation

quoctruong
Copy link
Contributor

When using the debugger in remote attach mode, users have to use cwd instead of program. This can be confusing so we should show a warning message.

Copy link
Contributor

@ramya-rao-a ramya-rao-a left a comment

Choose a reason for hiding this comment

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

Any idea why the whole file is appearing in the diff?

* Show warningMessage to the user if warningCondition is met.
* If the user chooses not to see the warning again, sets it to the global state.
*/
private showWarning(warningKey: string, warningMessage: string, warningCondition: boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If warningCondition is true, then the entire code path in this method is a no-op. It would be easier to read the code if this condition was checked outside and this function called only if it were true

Copy link
Contributor

Choose a reason for hiding this comment

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

Also would suggest renaming warningKey to ignoreWarningKey

Copy link
Contributor Author

@quoctruong quoctruong left a comment

Choose a reason for hiding this comment

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

`Request type of 'launch' with mode 'remote' is deprecated, please use request type 'attach' with mode 'remote' instead.`);
}

if (debugConfiguration.request === 'launch' && debugConfiguration['mode'] === 'remote') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the request here be attach?

@quoctruong
Copy link
Contributor Author

@ramya-rao-a PTAL

@ramya-rao-a ramya-rao-a merged commit 28bab92 into microsoft:master Mar 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants