Skip to content

Make sticky messages work better on windows #28

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

Closed
c42f opened this issue Mar 12, 2020 · 11 comments · Fixed by #37
Closed

Make sticky messages work better on windows #28

c42f opened this issue Mar 12, 2020 · 11 comments · Fixed by #37

Comments

@c42f
Copy link
Member

c42f commented Mar 12, 2020

Now that people have started using this package, we're seeing that some unfortunate workarounds are required. For example, the iswindows check here: https://github.com/JuliaDiffEq/DiffEqBase.jl/pull/454/files.

There seem to be a few options:

  • Use a combination of printing blank lines to force scrolling, followed by backward terminal cursor movement commands to simulate the behavior of the scroll region.
  • Whatever ConsoleProgressMonitor does which makes it work ok-ish on windows
  • Use some direct (synchronous) write hack which would bypass libuv and allow us to set the terminal scroll region on windows, regardless of libuv's attempt to intercept?
  • Longer term, now that win,tty: Added set cursor style to CSI sequences libuv/libuv#1884 has merged it might be the time to look into fixing libuv's handling of the scroll region escape codes. Unfortunately that's got a long lead time and won't fix things on older Julia versions.
@tkf
Copy link
Collaborator

tkf commented Mar 12, 2020

  • Whatever ConsoleProgressMonitor does which makes it work ok-ish on windows

ConsoleProgressMonitor simply wraps a small part of ProgressMeter.jl API. It doesn't even do "multiline erase" thing so all we use is \r. But my guess is that this would mean that we have to maintain two distinct logger types (i.e., the one doesn't use scroll region).

So, I think the most reasonable first attempt would be synchronous write that bypasses libuv. Maybe the first option is good too, but I'm guessing this (the third option) requires less code.

@c42f
Copy link
Member Author

c42f commented Mar 12, 2020

Excellent, thanks for sharing that context. I've got a windows 10 VM handy, I'll see what I can do.

@c42f
Copy link
Member Author

c42f commented Mar 13, 2020

Ok, it seems like simply bypassing libuv is not enough. It appears that Julia's modified libuv is preventing this from working at a deeper level by talking to the windows console API. A snippet like the following starts to make things somewhat-work, though I'm not sure what the other consequences of this hacking are.

const BOOL = Cint
const DWORD = Culong
const LPDWORD = Ptr{DWORD}
const HANDLE = Ptr{Cvoid}

function fix_console_mode()
    ENABLE_VIRTUAL_TERMINAL_PROCESSING = 0x0004
    STD_OUTPUT_HANDLE = reinterpret(DWORD, Int32(-11))
    hConsole = ccall(:GetStdHandle, HANDLE, (DWORD,), STD_OUTPUT_HANDLE)
    consoleMode = Ref{DWORD}(0);
    ccall(:GetConsoleMode, BOOL, (HANDLE, LPDWORD), hConsole, consoleMode)
    ccall(:SetConsoleMode, BOOL, (HANDLE, DWORD), hConsole,
          consoleMode[] | ENABLE_VIRTUAL_TERMINAL_PROCESSING)
end

This is based on @musm's code at
JuliaLang/julia#31491
JuliaLang/libuv#2

@tkf
Copy link
Collaborator

tkf commented Mar 14, 2020

Thanks a lot for looking into this. It's unfortunate that this is not going to be an easy hack...

By the way, option 1 may not be so bad in the sense that it can also be used as a workaround for #22; i.e., a TerminalLogger inside a subprocess can also use this "simulated scroll region" mode. Though I think this helps us only for the first-level subprocess.

@c42f
Copy link
Member Author

c42f commented Mar 16, 2020

Hmm. We could try to call fix_console_mode() as part of initializing StickyMessages. Reading the libuv source, it seems that libuv tries to parse ANSI codes itself and translate them into calls to the underlying windows console APIs. For example, color codes eventually turn into calls to SetConsoleTextAttribute.

Calling fix_console_mode() ourselves would result in two layers of parsing for text which goes via libuv: one done by libuv, and another by the windows internals after WriteConsole is called. Maybe that's ok, it depends on how much state libuv is tracking independently of the underlying windows API. Not much, it appears:

https://github.com/libuv/libuv/blob/288a06727b7ef74283369606d2002947d62d4ed0/src/win/tty.c#L1558

Alternatively maybe we just need to call whatever console API can be used to simulate the scroll region (DECSTBM from VT100). There's ScrollConsoleScreenBuffer but it appears that actually causes scrolling (not sets the scroll region). The code which deals with this in the windows terminal looks like the escape code might be the only way to get this functionality:

https://github.com/microsoft/terminal/blob/ae71dce2ca32c62399652ed19ccd9a32c167338d/src/host/getset.cpp#L1383

@musm
Copy link
Contributor

musm commented Jul 23, 2020

Should be fixed in julia now 😄

@musm
Copy link
Contributor

musm commented Jul 23, 2020

I don't really know the full main issue here but I do think it's probably fixed on the nightly builds. Would be great to check and confirm!

@visr
Copy link
Contributor

visr commented Jan 4, 2021

What are the main things to look at for this issue? I'd like to use these sticky messages for progress bars, so I went ahead and removed the iswindows here:
https://github.com/c42f/TerminalLoggers.jl/blob/c24a6bdda2ee376bb9f501b68ee7b2aea094f3a0/src/StickyMessages.jl#L21-L26

And did some tests with a simple @progress loop with @info inside, on 1.5.3 and 1.6.0-DEV, and the old cmd.exe and a newer Alacritty+PS.

The updating in place part worked at both julia versions I tried. Now I see that commits bumping libuv were backported to 1.5.3, so that is nice.

In cmd.exe if I manually scroll around during a progress bar things can get a little funky, on both julia versions:
image

But this does not happen in my Alacritty shell (I can't test on Windows Terminal unfortunately).

Is this good enough to remove the iswindows exclusion? I guess for VERSION 1.5.3 or higher?

@musm
Copy link
Contributor

musm commented Jan 4, 2021

@visr Agreed, this is fixed as long as you use a modern terminal aka Windows Terminal.

@c42f
Copy link
Member Author

c42f commented Jan 4, 2021

Is this good enough to remove the iswindows exclusion? I guess for VERSION 1.5.3 or higher?

Sounds great 👍

visr added a commit to visr/TerminalLoggers.jl that referenced this issue Jan 4, 2021
This now works due to new libuv patches landing in recent julia versions.

Fixes JuliaLogging#28
@jaakkor2
Copy link

Bump. Would #37 need more testing to get it merged and new version to be tagged?

I have tested https://diffeq.sciml.ai/stable/features/progress_bar/#Using-Progress-Bars-Outside-Juno on Win10 REPL using Julia v1.5.3, v1.6.0-beta1.0 and Windows Terminal and default cmd.exe prompt. Seems to work well for DiffEq progress bar purposes.

@c42f c42f closed this as completed in #37 Jan 12, 2021
c42f pushed a commit that referenced this issue Jan 12, 2021
This now works due to new libuv patches landing in recent julia versions.

Fixes #28
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 a pull request may close this issue.

5 participants