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

Notebook output must support more ANSI sequences #118833

Open
jrieken opened this issue Mar 12, 2021 · 16 comments
Open

Notebook output must support more ANSI sequences #118833

jrieken opened this issue Mar 12, 2021 · 16 comments
Assignees
Labels
feature-request Request for new features or functionality notebook-output
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Mar 12, 2021

follow up from #116416 (comment) and other discussions

Kernels often print stdout/stderr content with the assumption of being connected to a terminal. With that mindset they send ANSI sequences. Some of those sequences we support, like colors. We current don't support the sequence that clears the screen and that forces the jupyter extension to employ funny workarounds: they scan all output for that sequence, remember that so that they can clear the output before appending the next chunk. Things would be much simpler for them if we would simply support that ANSI sequence.

@jrieken jrieken added this to the March 2021 milestone Mar 12, 2021
@jrieken
Copy link
Member Author

jrieken commented Mar 12, 2021

cc @DonJayamanne for the sequence in question

@DonJayamanne
Copy link
Contributor

I'll update this issue with samples of the special characters that we support today.

@DonJayamanne
Copy link
Contributor

DonJayamanne commented Mar 12, 2021

Here are the special cases we have today (3 of them), anything beyond these would be an added bonus for anyone using VSCode to render streams.

Control Characters reference https://en.wikipedia.org/wiki/ANSI_escape_code#Control_characters

^ C0 Abbr Name Effect
^H 8 BS Backspace Moves the cursor left (but may "backwards wrap" if cursor is at start of line). Used here in our code https://github.com/microsoft/vscode-jupyter/blob/b368ea9326c13d6497e0f070d89d810d4ff86416/src/datascience-ui/common/index.ts#L94-L94
^M 0xD CR Carriage Return Moves the cursor to column zero. Used here in our code https://github.com/microsoft/vscode-jupyter/blob/b368ea9326c13d6497e0f070d89d810d4ff86416/src/datascience-ui/common/index.ts#L105-L105

Control Sequence reference https://en.wikipedia.org/wiki/ANSI_escape_code#CSI_(Control_Sequence_Introducer)_sequences

CSI Character Abbr Name Effect
CSI n A CUU Cursor Up Moves the cursor n (default 1) cells in the given direction. If the cursor is already at the edge of the screen, this has no effect, Used here https://github.com/microsoft/vscode-jupyter/blob/77d6162044e691085fb90dc12112ede8951b6ef7/src/client/datascience/jupyter/kernels/cellExecution.ts#L658-L658

@DonJayamanne
Copy link
Contributor

FYI - In our older webview we have support for general ANSI color codes as well.

@jrieken jrieken added notebook-output feature-request Request for new features or functionality labels Mar 16, 2021
@jrieken jrieken modified the milestones: March 2021, April 2021 Mar 21, 2021
@jrieken jrieken modified the milestones: April 2021, May 2021 Apr 23, 2021
@jrieken jrieken modified the milestones: May 2021, June 2021 May 31, 2021
@jrieken jrieken removed their assignment Jun 16, 2021
@jrieken jrieken removed this from the June 2021 milestone Jun 16, 2021
@rebornix rebornix removed the notebook label Oct 21, 2021
@bpasero bpasero added this to the Backlog milestone Aug 9, 2022
@bpasero
Copy link
Member

bpasero commented Aug 9, 2022

Moved to Backlog since this has no owner, please change as appropriate.

@roblourens
Copy link
Member

@DonJayamanne did you implement this as part of that streaming output perf issue?

@davidanthoff
Copy link
Contributor

Bump, I would also be interested in a status update on this for the Julia extension :)

@DonJayamanne
Copy link
Contributor

@davidanthoff please could you share more information about the specific ANSI codes that Julia extension would need support for
Thanks

@T-256
Copy link

T-256 commented Jul 16, 2023

I just noticed https://code.visualstudio.com/updates/v1_80#_terminal-output-support last day, Is it possible to have terminal as notebook output renderer? (As separated extension)

@davidanthoff
Copy link
Contributor

@DonJayamanne so I think in general the more the better, at the end of the day any package might just assume that things are run in a terminal that supports a lot of stuff.

Having said that, there is one extremely popular package in the Julia ecosystem that shows progress bars that is really broken at the moment, and if we just added support for the sequences that are needed there it would go a very, very long way to improve things. The specific sequences that are need are ESC [K, ESC [An (where n is a number) and ESC [A and generally support for \r. More details over at timholy/ProgressMeter.jl#271.

@MilesCranmer
Copy link

MilesCranmer commented Aug 29, 2023

Putting in another vote of support for this. Relevant ProgressBars.jl issue: cloud-oak/ProgressBars.jl#49. ProgressBars.jl needs basically the same ANSI sequences as ProgressMeter.jl (only difference is it also needs vertical movement via [1A and [1B):

erase_to_end_of_line(output_stream::IO) = print(output_stream, "\033[K")
move_up_1_line(output_stream::IO) = print(output_stream, "\033[1A")
move_down_1_line(output_stream::IO) = print(output_stream, "\033[1B")
go_to_start_of_line(output_stream::IO) = print(output_stream, "\r")

It's used as the main printing utility for the Python package pysr (1.4k⭐️) and Julia package SymbolicRegression.jl (450⭐️). Currently these packages cannot use progress bars in notebooks, so they have to fall back to regular printing.

@davidanthoff
Copy link
Contributor

@DonJayamanne and @amunger just wanted to check in on this again, this is probably the number one pain point I have right now with the notebook stuff for Julia :) Is there anything from our end we could do to help move this forward? At least the limited set of ANSI sequences I indicated above? Thanks :)

@DonJayamanne
Copy link
Contributor

@davidanthoff sorry there hasn't been any update since,
lets wait for @amunger to get back

@amunger
Copy link
Contributor

amunger commented Mar 6, 2024

The current limited escape codes we handle are handled manually here with some related tests.

We should be handling this more holistically, and I think there was a plan to do just that, but I'm not sure when we'll be able to get around to that. If you see a somewhat straightforward way to add in the set of escape codes that you need, I'd be happy to help review a contribution.

@DonJayamanne
Copy link
Contributor

@davidanthoff I just tried the following code and it works as expected

p = Progress(10)
for i in 1:10
    sleep(0.5)
    next!(p)
end

Also tried almost all the samples here https://github.com/timholy/ProgressMeter.jl and they all work as expected.
FYI = I'm using the Julia kernel isntalled via IJulia

It only fails to work when using the Julia VS Code extension
Is it not possible to update the Julia VS Code extension accordingly.
Given that it works natively when using IJulia, it feels like updating Julia VS Code extension might be one of the ways to get this addressed.

@davidanthoff
Copy link
Contributor

I looked a bit more, and I think if we could fix julia-vscode/julia-vscode#3561 it would already be great progress. @DonJayamanne or @amunger if you could point me in the right direction for that issue it would be great!

That still won't cover all cases: as soon as ProgressMeter.jl has a progress bar that also outputs additional info that would trigger a multi-line output (essentially next!(p, showvalues=[(:foo, 4), (:bar, 6)]) style) then things start to look bad on any Jupyter implementation because then ProgressMeter.jl resorts to just clearing the entire cell output with every update, so any output before the progress bar is lost. While it would obviously be nice to fix that also (and more complete support for ANSI sequences would be the solution there) that seems maybe less urgent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality notebook-output
Projects
None yet
Development

No branches or pull requests

10 participants