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

Support lazy loading object values from toString() #401

Merged
merged 9 commits into from Mar 24, 2022
Merged

Conversation

CsCherrYY
Copy link
Contributor

@CsCherrYY CsCherrYY commented Mar 14, 2022

requires: microsoft/vscode-java-debug#1134

specification: https://microsoft.github.io/debug-adapter-protocol/specification#Types_VariablePresentationHint

demo:

lazy

more details can be found in microsoft/vscode#135147, and there is still a UX discussion.

@@ -94,6 +95,24 @@
}

VariableProxy containerNode = (VariableProxy) container;

if (containerNode.getReferencedVariableId() != null && DebugSettings.getCurrent().showToString) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest to extract the following block to a private method like resolveLazyValue(variable) for more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in c8366d5

String valueString = variableFormatter.valueToString(variable, options);
String detailString = VariableDetailUtils.formatDetailsValue(variable, containerNode.getThread(), variableFormatter, options,
evaluationEngine);
return new Types.Variable("", valueString + " " + detailString, "", referenceId, "");
Copy link
Contributor

Choose a reason for hiding this comment

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

When running it on an old vscode, you will find variable name info is missing.

image

Copy link
Contributor

@testforstephen testforstephen left a comment

Choose a reason for hiding this comment

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

the latest commit works.

@beckermarc
Copy link

Is it somehow possible to disable this? I now have to do a lot of clicks to view the desired information during debugging...

@CsCherrYY
Copy link
Contributor Author

@beckermarc you can set configuration java.debug.settings.showToString to false to disable this.

@beckermarc
Copy link

Thanks. However that seems to disable the whole feature. I'd prefer to only disable the lazy loading. Is that also possible?

@CsCherrYY
Copy link
Contributor Author

Thanks. However that seems to disable the whole feature. I'd prefer to only disable the lazy loading. Is that also possible?

@testforstephen any comments?

@testforstephen
Copy link
Contributor

Make sense to have a setting to turn it off.

@beckermarc
Copy link

I think my main concern with this change is that it also seems to hide more information than just the toString() value:

Here is one example before the version that introduced this change:
Before

Here is the same variable with the new version. Note that I also don't see the class anymore and are no longer made aware of the fact that I can expand this variable further:
After (not loaded)

After clicking the ellipsis I get the same information as before:
After (loaded)

Especially when trying to get an overview over all variables in the current scope this requires me to do a lot more clicks. I never ran into issues with long running toString() methods in my projects so far, so it would be great to be able to disable this from my perspective.

@testforstephen
Copy link
Contributor

yes, it would be better to display the class name during lazy loading. This is an improvement we also reported back to VS Code core team. microsoft/vscode#135147 (comment)

@beckermarc
Copy link

cool, thanks for your work!

@veastark
Copy link

Please include an option to disable the lazy loading and return to the old method of displaying variable values. With this addition debugging has become very painful.

@testforstephen
Copy link
Contributor

VS Code has added a setting debug.autoExpandLazyVariables to control whether to auto expand lazy variable. That will be released in next VS Code version. Currently you can try it with VS Code Insider first.

@testforstephen
Copy link
Contributor

FYI. The latest VS Code 1.67.0 has provided a setting debug.autoExpandLazyVariables to allow you to expand the variable full value by default.

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.

None yet

4 participants