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

Debug console uses unexpected text colours for some strings #34043

Closed
DanTup opened this issue Sep 8, 2017 · 11 comments
Closed

Debug console uses unexpected text colours for some strings #34043

DanTup opened this issue Sep 8, 2017 · 11 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded

Comments

@DanTup
Copy link
Contributor

DanTup commented Sep 8, 2017

Add the following code in an extension:

console.log('true vs true');
console.log('true vs false');
console.log('false vs true');
console.log('false vs false');

When it runs, the colours of the output lines are inconsistent:

Debug colours

Looking in the dev tools, it seems the blue lines have boolean in their CSS class which is missing from the white lines. It seems strange that true vs false and false vs true are categorised differently.

Dev tools

@vscodebot vscodebot bot added the debug Debug viewlet, configurations, breakpoints, adapter issues label Sep 8, 2017
@weinand
Copy link
Contributor

weinand commented Sep 8, 2017

@isidorn why do we try to colorise stuff in the debug console?
I remember that I've already filed an issue about numbers being colorised in strange ways.

@weinand weinand added the bug Issue identified by VS Code Team member as probable bug label Sep 8, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 12, 2017

@weinand currently we use some simple regex heuristics to try to colorise booleans, numbers and strings.
Also we colorise some ANSI output.

I can remove all of these colors but not sure if it will upset the users.
And yes I agree that the colorisation is a bit flaky.
Suggestions on what to do here are very welcome.

@weinand
Copy link
Contributor

weinand commented Sep 12, 2017

@isidorn please remove the regex heuristics for colorising because it is surprising and mostly wrong. But keep the ANSI coloring because it is deterministic and expected.

@isidorn isidorn added this to the September 2017 milestone Sep 12, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 12, 2017

@weinand makes sense. Do you get a sense that it is mostly wrong also in the watch and variables pane since we use the same heuristic there. Should it also be removed there?
IMHO it is giving some additional info in those panes and nobody complained so we should keep the heuristic for the variables and watch

screen shot 2017-09-12 at 15 50 57

@DanTup
Copy link
Contributor Author

DanTup commented Sep 12, 2017

My 2¢...

I was surprised to see colours in the debug output. I'm not against it, though if it's kept it should be reliable.

If the same regex is being used for the watch window, then even if colours are removed from the debug console, then I think the regex needs fixing, since it'll presumably also color the string "false vs false" and "false vs true" differently?

@weinand
Copy link
Contributor

weinand commented Sep 12, 2017

If we want to continue to support regexp-based "value colorisation", we should only do it where we control and understand the underlying syntax. This is true for the variables and watches view and for structured objects in the debug console since all are trees.
But we should not try to do it where we have no clue what the underlying structure is. This applies to to all "string" output in the debug console. Here only the ANSI colours should be honoured.

For the trees we should make sure that the regexps only match simple values like true and falsebut not something like true and false or a trueembedded in a string.

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

Thanks.
Now we do not colorize in repl outside of a tree.

@DanTup I have checked the boolean regex and it looks fine to me. I think the difference in coloring comes from the fact that those output elements came in a different order from the adapter. Which means that part of tha string first came which passed the regex check making it blue.
Though this will no longer happen since I removed color for that case in the repl

@DanTup
Copy link
Contributor Author

DanTup commented Sep 13, 2017

@isidorn I don't understand the explanation for it happening (the output is in the same order as the console.logs?) but I don't know the internals of how the data gets back to the debug output window. If it makes sense to you (and is no longer an issue anyway), all sounds good to me 👍

@isidorn
Copy link
Contributor

isidorn commented Sep 13, 2017

@DanTup no it does not make 100% sense to me either, I was just speculating as to what might have been the cause. Since the adapter can send the output events chunked to vscode (I did not investigate further)

@roblourens
Copy link
Member

It would be nice to keep colorization for 'evaluate' requests, since the result has a 'type'. node2 doesn't set the type... it could though.

image

vs previously

image

@roblourens roblourens added the verified Verification succeeded label Sep 27, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 28, 2017

@roblourens please file a feature request to me to respect the type in the debug console for colorisation

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants