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

"Colour wrapping" #2161

Closed
gvanem opened this issue Mar 12, 2022 · 9 comments
Closed

"Colour wrapping" #2161

gvanem opened this issue Mar 12, 2022 · 9 comments
Labels
wontfix A feature or bug that is unlikely to be implemented or fixed.

Comments

@gvanem
Copy link

gvanem commented Mar 12, 2022

What version of ripgrep are you using?

ripgrep 13.0.0, -SIMD -AVX (compiled)

How did you install ripgrep?

Built it myself by cargo build --all --release --features pcre2

What operating system are you using ripgrep on?

Win-10, 21H1, ver. 19043.1586.

Describe your bug.

As part of a pygrep.bat file I use RipGrep like:

setlocal
set RG_COLORS=--colors "match:fg:white" --colors "match:bg:red" ^
              --colors "path:fg:green"  --colors "path:style:intense" ^
              --colors "line:bg:blue"   --colors "line:style:intense"   --colors "line:fg:white" ^
              --colors "column:bg:blue" --colors "column:style:intense" --colors "column:fg:green"

::
:: handle option '-c' to set RG_OPTIONS=-i' etc. etc.
::
rg.exe --after-context=1 --mmap --fixed-strings f:\ProgramFiler\Python27\test %RG_OPTIONS% %RG_COLORS%  %*

With the Windows console width set to 120, a command like pygrep.bat IP2Location, shows this:
pygrep-rg-colours

the red colour bleeds over to the next line marking a non-match.

What is the expected behaviour?

With the WinConsole width set to e.g. 130, there is no problem. A cls & pygrep.bat IP2Location, shows this:
pygrep-rg-colours-130

In lack of a better description, I named this "Colour wrapping". I've no idea what's the real issue here.
But this depends on which Y-position the .bat-file was invoked on. It's as if a 1 line scroll when "match:bg:red" is in
effect causes this bleeding. Since if I increase the console height to avoid a scroll, then there's no bleeding.

PS. I wrote that netstat.py script using psutil.

@BurntSushi
Copy link
Owner

This feels more like a rendering bug than a problem with ripgrep. Regardless, I'm not a Windows user. Someone else will have to debug this.

@gskt17
Copy link

gskt17 commented Mar 18, 2022

I'm on windows, so I can look into this. That said, I'm also not an expert on Windows. My guess would be this has something to do with the way the Windows console writes wrapped lines; it looks to me like what happened was that after writing the i in IP2Location, the cursor moved down and then back to the beginning of the line, setting the background and foreground colors at each location along the way. I suspect that this is documented behavior somewhere? I think the fix will probably go in termcolor and not rg, if a fix is even possible.

@gskt17
Copy link

gskt17 commented Mar 18, 2022

It seems like this issue is known

It may be related to these resizing bugs:

Clearing the remainder of the line when writing a newline (per JanDeDobbeleer/oh-my-posh#65) will apparently produce the correct behavior.

@BurntSushi: Apparently, Windows already has VT100 (ANSI) escape sequence support. They still require accessing the Windows console API to enable, but if I'm reading the docs correctly, they should work the same way any other OS does. I think they probably exist because of the Linux subsystem in Windows 10.

@gvanem
Copy link
Author

gvanem commented Mar 19, 2022

@gskt17 Your links mentions PowerShell for the most part. I forgot to say I'm using 4NT as my shell.
So the bug is in probably in the underlaying conhost process.

@gskt17
Copy link

gskt17 commented Mar 29, 2022

@gvanem Sorry for late reply, but the bug seems to be present in any shell on Windows, so I think you're correct.

@BurntSushi What would you say the best way to insert a line clear just before a newline is written in the output functions? I think I've found a place but I can't say for sure.

I've hit another snag, too: Windows API doesn't offer the equivalent to "clear to remainder of line," only VT mode does, so I would need to manually change the remainder of the line on wrapping. I'm going to see about forking termcolor to add some of that functionality. Also, I should note that the bug only takes place on write and resizing the console immediately fixes it.

@BurntSushi
Copy link
Owner

To be honest, I just don't really have the bandwidth right now to dig into this. The Windows console API is indeed quite limited. On the other hand, it's been quite some time (years now) since Windows supported ANSI color escapes. So if you fix it for ANSI escapes, that might be enough.

The right place for this to go is indeed probably in termcolor, likely all the way down in the lowest levels of the write routine. It should only happen on Windows, and it might be a good idea to provide a way to disable it in the crate API.

Another option here, honestly, is to not fix it. This really seems like a bug in how Windows consoles handle this stuff to me. But if the fix is to do a search for \n on every write call, then I guess it's just yet another penalty that Windows users pay. (They already pay a fair amount. ripgrep is generally much quicker on Linux/macOS in my experience.)

@ninmonkey
Copy link

Another option here, honestly, is to not fix it... So if you fix it for ANSI escapes

The Short Version is that Microsoft says:

"For all new and ongoing development on Windows, virtual terminal sequences are recommended as the way of interacting with the terminal."

I didn't see this cleary summarized, so I added extra information so that they may link to it. place.

Windows xterm compatibility

Name xterm compatible?
Windows Console Host Yes for Windows 10 and higher
Windows Terminal Yes
Supports 24bit color and utf8 support
Is the default in win11

Quotes from the Documentation

That red one sounds like @gvanem OP's bug, and fits with the StackOverflow threads about bugs when resizing the window.


@gskt17 should work like any other platform

The last link had a little bit on the origin

issues

@BurntSushi

Windows users pay. (They already pay a fair amount. ripgrep is generally much quicker on Linux/macOS in my experience.)

Fairly recently -- maybe the last 5?, 10? years -- Microsoft has turned around. Instead of competing, they started integrating open source projects. Powershell being both open source and cross platform is pretty neat. ripgrep works great on windows.

@BurntSushi
Copy link
Owner

I agree it works great. I'm speaking from experience benchmarking equivalent corpora with similar CPUs. Windows tends to be quite a bit slower.

It has been only a couple years since I've performed said benchmarks. And I do not claim to know why there is a difference, although I have a variety of guesses. Windows requires additional work in some cases with interacting with its system APIs (not console APIs).

@BurntSushi
Copy link
Owner

This issue does not appear actionable on my end.

@BurntSushi BurntSushi closed this as not planned Won't fix, can't repro, duplicate, stale Nov 24, 2023
@BurntSushi BurntSushi added the wontfix A feature or bug that is unlikely to be implemented or fixed. label Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wontfix A feature or bug that is unlikely to be implemented or fixed.
Projects
None yet
Development

No branches or pull requests

4 participants