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

Clearing console in node is not clearing it, instead dumping weird characters #27389

Closed
jpike88 opened this issue May 28, 2017 · 20 comments
Closed
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@jpike88
Copy link

jpike88 commented May 28, 2017

  • VSCode Version: 1.12.2
  • OS Version:MacOS Sierra

Steps to Reproduce:

Install clear using 'npm clear'

Debug a node app that has code like this:

var clear = require('clear');

console.log("Stuff");

clear(true);

console.log("This is all that should display")

Instead, I see stuff like this (notice odd characters):

screen shot 2017-05-28 at 5 06 23 pm

@roblourens
Copy link
Member

That's a control code for clearing the terminal, that isn't supported by the console.

@roblourens roblourens added debug-console feature-request Request for new features or functionality labels May 29, 2017
@isidorn
Copy link
Contributor

isidorn commented May 29, 2017

The node2 debug adapter could exectue the following command on the vscode side once it detects this special control code
"command": "workbench.debug.panel.action.clearReplAction"

The language agnostic UI should not solve this special node case

@isidorn isidorn assigned roblourens and unassigned isidorn May 29, 2017
@roblourens
Copy link
Member

That makes sense. We don't have a way for the debug adapter to send a message to the extension host side right now, right? Just the other way around?

@roblourens
Copy link
Member

The console supports the color control codes, right? Should this fall into the same category? Might be relevant for the powershell extension, etc.

@isidorn
Copy link
Contributor

isidorn commented May 31, 2017

It can fall in the same category as the colors if there is one universal control character that is used across languages to clear the console.

Yes, we currently do not have a mechanism for the debug adapter to send messages to the extension host, just the other way around. Though we do have the notion of the adapter sending a request to VSCode, more details here. This could easily be enhanced to cover the case of debug adapter being able to request some commands to be executed.

@weinand thoughts?

@weinand
Copy link
Contributor

weinand commented May 31, 2017

If there is a standard (ANSI) code for clearing a terminal we should try to support this for the debug console.

I checked and there is an "Erase Display" escape sequence (see: http://ascii-table.com/ansi-escape-sequences.php)

@isidorn
Copy link
Contributor

isidorn commented May 31, 2017

Thanks @weinand then it makes sense to do it on the vscode side. Assigning to me

@isidorn isidorn assigned isidorn and unassigned roblourens May 31, 2017
@isidorn isidorn added this to the June 2017 milestone May 31, 2017
@isidorn isidorn closed this as completed in 38743ee Jun 6, 2017
@isidorn
Copy link
Contributor

isidorn commented Jun 6, 2017

We now support this.
Note that in this case the npm clear module also sends hte [0f ansi escape code after the clear one. Since we do not specially treat that the console does not get fully empty since it contains this extra part. This extra code is used for setting the location of the screen which we do not support

@isidorn isidorn added the verification-needed Verification of issue is requested label Jun 6, 2017
@jpike88
Copy link
Author

jpike88 commented Jun 6, 2017

@isidorn It would be nice to at least omit unsupported escape codes to prevent them from polluting console output. Or is this something I should be fixing with the clear module directly?

@weinand
Copy link
Contributor

weinand commented Jun 6, 2017

VS Code should omit unsupported escape codes.

@isidorn
Copy link
Contributor

isidorn commented Jun 6, 2017

I agree that vs code should omit unsopported escape codes however we currently do not have a nice list of what we support and what we do not and we only have a utility method to remove all ansi escape code.

Code pointers:
https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/parts/debug/electron-browser/replViewer.ts#L276
https://github.com/Microsoft/vscode/blob/master/src/vs/base/common/strings.ts#L662

PRs / feature requests welcome

@jens1o
Copy link
Contributor

jens1o commented Jun 6, 2017

What does vscode support? I think I can handle that.

@isidorn
Copy link
Contributor

isidorn commented Jun 6, 2017

@jens1o you can see what is being used in the replViewer method handleAnsi which I have linked above

@jens1o
Copy link
Contributor

jens1o commented Jun 6, 2017

Okay, thanks.

@jens1o
Copy link
Contributor

jens1o commented Jun 6, 2017

Before I create a pr which is wrong, is this right? jens1o@9bc68d7

@isidorn
Copy link
Contributor

isidorn commented Jun 6, 2017

There are a couple of things wrong with that approach

RemoveUnsopported is only debug specific since the debug framework is currently not supporting them, it is not like we have a general VSCode concenzus that they are not supported.
Due to that this method should not live in common/strings.ts, but in some more precise debug related location.

CopyAllACtion shuold not be touched since it shuold remove all ansi escape codes, no matter if they are supported or not.
Same holds true for all other methods you replaced.
This removal of unsuported ansi codes should only be done in debugService logToRepl method imho.

@jens1o
Copy link
Contributor

jens1o commented Jun 6, 2017

Okay, thanks for the feedback.

@jens1o
Copy link
Contributor

jens1o commented Jun 6, 2017

I hope this fits your needs: master...jens1o:jens1o-strip-unsupported-ansi

@weinand weinand added the verified Verification succeeded label Jun 28, 2017
@weinand
Copy link
Contributor

weinand commented Jun 28, 2017

I've verified that the "Erase Display" escape sequence works (but the unrecognised sequence '[0f' still shows up).

@jens1o please create a real PR if you want to have us look into your fix.

BTW, I noticed in your change that the way you construct the regular expression "UNSUPPORTED_ANSI_REGEX" looks fishy to me. The result is a string and not a reg exp. So the removeUnsupportedAnsiCodes helper function does not work the way you think it should work.

Have you verified that the code from the first comment in this issue really shows no '[0f' at the beginning of the line?

@weinand weinand added the debug Debug viewlet, configurations, breakpoints, adapter issues label Jun 28, 2017
@jens1o
Copy link
Contributor

jens1o commented Jun 28, 2017

Unfortunately no, I can't start compiling in an acceptable time, so I can't contribute on this topic.

@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
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants