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

Can't copy the values of debugging elements such as exception messages #10210

Closed
chuckries opened this issue Aug 5, 2016 · 18 comments
Closed
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues
Milestone

Comments

@chuckries
Copy link
Member

From @CarlosTorrecillas on August 5, 2016 8:24

Environment data

dotnet --info output:
.NET Command Line Tools (1.0.0-preview2-003121)

Product Information:
Version: 1.0.0-preview2-003121
Commit SHA-1 hash: 1e9d529bc5

Runtime Environment:
OS Name: Mac OS X
OS Version: 10.11
OS Platform: Darwin
RID: osx.10.11-x64

VS Code version: 1.3.1
C# Extension version: 1.3

Steps to reproduce

  • Create a .NET Console app
  • Add the code to throw an exception
  • On the debugging section try copy the value of either the message or the stacktrace

Expected behavior

  • I would expect the value to be copied in the clipboard

Actual behavior

  • I get the message "error CS1012: Too many characters in character literal"

Copied from original issue: dotnet/vscode-csharp#639

@CarlosTorrecillas
Copy link

Ive been told to raise the issue in the two repos so I don't know where it belongs to

@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Aug 9, 2016
@weinand
Copy link
Contributor

weinand commented Aug 9, 2016

@chuckries Sounds like a problem of the C# debug extension because it generates the "error CS1012:...".

@weinand
Copy link
Contributor

weinand commented Aug 9, 2016

@chuckries saw your comment on dotnet/vscode-csharp#639. So what exactly is the problem with VS Code that results in the CS1012 error on your side?

@isidorn
Copy link
Contributor

isidorn commented Aug 23, 2016

VSCode will try to evaluate the variable before it copies it's value to the clipboard here
This was originally done because the node debugger cuts off long string value unless they are evaluated.

So the bottom line issue here is that the debug adapter can not evalute the long string that is trying to be copied over.
Not sure what we can do on the vscode side here.
@chuckries @weinand suggestions welcome

@chuckries
Copy link
Member Author

I'll take a look at this from the debugger side. I didn't realize an evaluation was performed on copy.

@chuckries
Copy link
Member Author

I think we need some way to opt out doing an evaluation on 'Copy Value' and just hand back the value sitting in the UI. As it stands, for copying Message out of $exception in the following screenshot:
image
We are asked to evaluate the expression $exception [Exception]['Message [string]'], which is completely meaningless to us. The error being seen when pasting is coming from the compiler failing to evaluate. We have the ability to evaluate ($exception).Message, but I'm not sure how we could manipulate the evaluation string into that. As it stands we can't even copy an int value out of the locals window because the evaluation string we are given includes the type annotation: i [int]

@gregg-miskelly @caslan @wesrupert for visibility. Wes did the type annotating work on our side. I'm not sure if this is something we need to scrape back out or if VS Code can give us the identifier without providing the type annotation.

@chuckries
Copy link
Member Author

Another option would to pass a token to the variable that needs to be re-evaluated rather than sending a raw string. My understanding is that this token approach is used other places in the debugger UI.

@wesrupert
Copy link

If VS Code wants to perform this evaluation, the best method I see for giving us the context would be to give us the same data as we get for the setVariable command, i.e. a context (stackframe or variablesReference) and the name, and we can handle it from there.

@wesrupert
Copy link

Seeing as the underlying behavior is from a desire to get a "better" string out of the value, it would also be nice to have a separate context for this evaluate command, e.g. "context": "copy". Looks like it currently doesn't send a context at all, as trying to copy the args local in a sample app gives the following command: evaluate: {"expression":"args [string[]]","frameId":1000}.

@isidorn
Copy link
Contributor

isidorn commented Aug 24, 2016

Passing a "context":"copy" in this case makes sense to me.
@weinand opinion?

@gregg-miskelly
Copy link
Member

Personally I don't think 'context' is enough -- VS Code shouldn't attempt to concatenate children together to build the name of an expression as this isn't language-agnostic. As Wes suggested, I think the clean way to handle this problem is to add a new request (GetFullValue?) which works the same as setVariable. For debugAdapters that don't support GetFullValue, VS Code should default to copying the original value.

@isidorn
Copy link
Contributor

isidorn commented Aug 26, 2016

Assigning first to @weinand to investigate how to solve this on the debug protocl level, and once we come to a conclusion I can implement it on the vscode side.

@isidorn isidorn assigned weinand and unassigned isidorn Aug 26, 2016
@weinand weinand added this to the September 2016 milestone Aug 26, 2016
@weinand weinand modified the milestones: September 2016, October 2016 Sep 22, 2016
@weinand
Copy link
Contributor

weinand commented Oct 24, 2016

In the October release we've introduced a new attribute evaluateName on the Variable type (see microsoft/vscode-debugadapter-node#73).
This can be used in an evaluateRequest to copy the value of debugging elements to the clipboard.

@isidorn please use this new (optional) attribute when copying values to the clipboard.

@weinand weinand modified the milestones: November 2016, October 2016 Oct 24, 2016
@weinand weinand assigned isidorn and unassigned weinand Oct 24, 2016
@isidorn isidorn modified the milestones: October 2016, November 2016 Oct 24, 2016
@isidorn
Copy link
Contributor

isidorn commented Oct 24, 2016

Currently if the evaluateName is passed we will respect it on the vscode. If it is not passed we will still use our ugly heuristic for figuring out the evaluateName here

I propose that in October we support both but in the november release we drop the support for not specifiying the evaluateName. In that case if the evaluateName is not passed we will simply use the name of the variable. No critical feature is dependent on this and I would like to get rid of that ugly code thus I am all in for droping this in november.

@isidorn
Copy link
Contributor

isidorn commented Nov 2, 2016

@weinand is it fine if I drop our ugly heuristic for figuring out the evalute name of a variable this milestone. This will break the following scenarios for adapters that do not support evaluateName:

  • adding a child element to a watch expression via context menu
  • copying a value of a child element via context menu

I am +1 for dropping it since this is not a major breakage imho and each adapter can start supporting evaluateName to prevent this.

@weinand
Copy link
Contributor

weinand commented Nov 2, 2016

Since we did not even mention evaluateName in the October release notes, we cannot drop the fallback strategy in November. Neither node-debug nor node-debug2 have adopted this and their adoption is not yet on the November plan.

@isidorn
Copy link
Contributor

isidorn commented Nov 2, 2016

@weinand makes perfect sense. In the future let me know when node-debug or node-debug2 implement this so we can drop the hack.

@weinand
Copy link
Contributor

weinand commented Nov 2, 2016

I've created this issue for the adoption work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

No branches or pull requests

6 participants