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

Fixes #19580; added copy property path action to debug viewlet #43423

Merged
merged 2 commits into from
Feb 14, 2018

Conversation

ergunsh
Copy link
Contributor

@ergunsh ergunsh commented Feb 11, 2018

Fixes #19580

@msftclas
Copy link

msftclas commented Feb 11, 2018

CLA assistant check
All CLA requirements met.

static LABEL = nls.localize('copyPropertyPath', "Copy Property Path");

constructor(id: string, label: string, private value: any) {
super(id, label, 'debug-action copy-property-path');
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to pass the css class since this action does not have that, simply pass undefined as the third argument

@@ -34,6 +34,23 @@ export class CopyValueAction extends Action {
}
}

export class CopyPropertyPathAction extends Action {
Copy link
Contributor

Choose a reason for hiding this comment

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

CopyEvaluatePathAction is a better name imho

@@ -34,6 +34,23 @@ export class CopyValueAction extends Action {
}
}

export class CopyPropertyPathAction extends Action {
static readonly ID = 'workbench.debug.viewlet.action.copyPropertyPath';
Copy link
Contributor

Choose a reason for hiding this comment

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

Change id to workbench.debug.viewlet.action.copyEvaluatePath'

export class CopyPropertyPathAction extends Action {
static readonly ID = 'workbench.debug.viewlet.action.copyPropertyPath';
static LABEL = nls.localize('copyPropertyPath', "Copy Property Path");

Copy link
Contributor

Choose a reason for hiding this comment

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

I would change the label to "Copy Path"

@@ -179,6 +179,7 @@ class VariablesActionProvider implements IActionProvider {
const actions: IAction[] = [];
const variable = <Variable>element;
actions.push(new SetValueAction(SetValueAction.ID, SetValueAction.LABEL, variable, this.debugService, this.keybindingService));
actions.push(new CopyPropertyPathAction(CopyPropertyPathAction.ID, CopyPropertyPathAction.LABEL, variable));
Copy link
Contributor

Choose a reason for hiding this comment

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

Set Value and Copy Value should be first, So CopyEvaluatePathAction should come third

@isidorn
Copy link
Contributor

isidorn commented Feb 13, 2018

@ergun1017 I have commented directly in the code - mostly polish. Once you address all those I can merge this in. Thanks

@isidorn isidorn merged commit 67ea7f6 into microsoft:master Feb 14, 2018
@isidorn isidorn added this to the February 2018 milestone Feb 14, 2018
@isidorn
Copy link
Contributor

isidorn commented Feb 14, 2018

@ergun1017 good job, thanks a lot 🍻

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
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.

Debuggers: Add "copy path" to the context menu in the inspector
3 participants