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

"clear console" escape sequence is checked for debug console without the escape character #51245

Closed
roblourens opened this issue Jun 6, 2018 · 15 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues verified Verification succeeded
Milestone

Comments

@roblourens
Copy link
Member

From microsoft/vscode-debugadapter-node#185

https://github.com/Microsoft/vscode/blob/78d7d64c5d16b8d89d1dd2b0cc7fcbe966feb48b/src/vs/workbench/parts/debug/electron-browser/debugService.ts#L669

is checking literally [2J not \u001b[2J

@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues bug Issue identified by VS Code Team member as probable bug labels Jun 6, 2018
@weinand weinand added this to the June 2018 milestone Jun 6, 2018
@isidorn isidorn closed this as completed in e4a075f Jun 7, 2018
@isidorn
Copy link
Contributor

isidorn commented Jun 7, 2018

I have improved the check on that line such that it now looks in substrings, try it out and let me know how it works.
We have one central place for dealing with handling ANSI coloring - in case we get more ansi feature requests we could put all those there.
Code pointer https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/browser/debugANSIHandling.ts#L1

@roblourens
Copy link
Member Author

Thanks! Another suggestion, Chrome devtools shows a hint in the console after it's cleared, like this:

image

which is probably useful for knowing that there may have been output there previously. Should we consider something like that for vscode too?

@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2018

I think this is useful when the console is cleared using a control character, since it is unexcpected by the user.
When the console is cleared using a user action I would not show this text. I just tried chrome and it also behaves like this.
So I will implement something like that

@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2018

screen shot 2018-06-08 at 10 24 51

fyi @weinand

@roblourens
Copy link
Member Author

roblourens commented Jun 8, 2018

That sounds good. Especially because chrome devtools doesn't look for this escape code. Thanks.

@roblourens
Copy link
Member Author

roblourens commented Jun 8, 2018

One more thing :)

https://github.com/Microsoft/vscode/blob/19dcc2d86952dd01deb4162143b3eee15c1e48aa/src/vs/workbench/parts/debug/electron-browser/debugService.ts#L673

This is a literal backslash, you want '\u001b[2J', \u is a unicode escape sequence in JS.

@weinand
Copy link
Contributor

weinand commented Jun 8, 2018

@isidorn it would help if you would verify your code by actually running it once ;-)

E.g. by using https://www.npmjs.com/package/clear and running this:

var clear = require('clear');
clear();

isidorn added a commit that referenced this issue Jun 11, 2018
@isidorn
Copy link
Contributor

isidorn commented Jun 11, 2018

@roblourens thanks rob, I pushed a fix
@weinand thanks for the example, but I tried that and the clear does not send any console output to the vscode ux, thus I could not verify with that example (and still can not)

@weinand
Copy link
Contributor

weinand commented Jun 11, 2018

@isidorn So how should we verify this feature then?

Have you tried console.log('hello \u001b[2J world')?
Do you understand why npmjs.com/package/clear is failing?

@isidorn
Copy link
Contributor

isidorn commented Jun 11, 2018

@weinand yes the console log example works that is how I verified
npmjs clear is failing beacuse no console output statement is coming from the debug adapter. I did not investigate inside the debug adapter as to why that is happening. I can create an independent issue if that will help?

@weinand
Copy link
Contributor

weinand commented Jun 11, 2018

@isidorn there is no need to investigate inside the debug adapter.
If you step through the few lines of "clear()" you will see that the sequence is sent to "process.stdout.write" instead of "console.log".
To capture the ANSI clear sequence as Output events you have to add an "outputCapture": "std" to the launch config.

Try this and verify that "clear" works with your feature.

@isidorn
Copy link
Contributor

isidorn commented Jun 11, 2018

@weinand thanks, after changing "outputCapture": "std" I can verify that the "clears" works with our feature.

@JacksonKearl
Copy link
Contributor

Is the verification needed here that "Console was cleared" was printed? If so verified. But the original issue is about not checking for escape sequence correctly. Which I verified too I guess.

@JacksonKearl JacksonKearl added the verified Verification succeeded label Jun 28, 2018
@roblourens
Copy link
Member Author

Yeah, I guess there were two parts, is the console cleared, and does it print the message.

@JacksonKearl
Copy link
Contributor

the console is cleared even when the escape sequence is in the middle of a log string, and it does print the message, and it does log the remainder of the string after the escape sequence

@vscodebot vscodebot bot locked and limited conversation to collaborators Jul 22, 2018
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