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

v1.5.17 display problem in VS Code for wider resolutions #524

Closed
rokrupnik opened this issue Nov 27, 2023 · 7 comments
Closed

v1.5.17 display problem in VS Code for wider resolutions #524

rokrupnik opened this issue Nov 27, 2023 · 7 comments
Labels
bug Something isn't working

Comments

@rokrupnik
Copy link

rokrupnik commented Nov 27, 2023

OS: Windows 10
clink: 1.5.17
cmder:
VS Code: 1.83.1

description:
I use cmder as my local terminal client integration into VS Code, following this official guide.

When I start the terminal in an fullwidth mode (monitor resolution 2560 x 1440) the suggestions are jumbled up (multiple lines, some weird text in between), see screenshot.
However, when I make the terminal more narrow, everything starts to work ok, see screenshot.
The breaking point seems to be around the 2100px.
The same happens in PHPStorm (IntelliJ).

clink.log
clink set
clink info

@chrisant996
Copy link
Owner

@rokrupnik Thank you for reporting this, and for including the diagnostic info! And especially thank you for the additional info about a ~2100px breakpoint. (And for giving it a unique name "clink_rupnik.log" so it's separate in my Downloads folder! 🤩 I appreciate the attention to detail.)

When I replay the log file on my machine, it works fine. But, since my current monitor is only 1920px wide, I made the terminal width 306 characters to match yours from the log file. Maybe the actual pixel width is relevant, so I'll try it on a larger monitor later today.

It seems that the hyperlink escape codes are getting mishandled by the terminal when the terminal width is large. That seems strange, but I recently learned that terminals tend to garble escape codes when outputting long strings. And Clink outputs the entire prompt and input line as a single string in order to work around problems when resizing the terminal. It's beginning to look like it's a deadly tradeoff: either resizing doesn't work, or escape codes in long lines get garbled.

This definitely gives me a promising lead to follow up on later tonight, and I'll report back with my findings.

In the meantime, running clink set autosuggest.hint false will probably work around the issue.

@arogl
Copy link

arogl commented Nov 28, 2023

I have a similar issue in windows terminal preview while using oh-my-posh since upgrading to v1.5.17

Screenshot shows the double prompt and then on windows resize it goes back to a single prompt until the next command is entered

Terminal display issues

Windows Terminal info
image

clink info

version          : 1.5.17.288dce
session          : 17376
injected         : clink_dll_x64.dll
binaries         : C:\Program Files (x86)\clink
state            : C:\Users\arogl\AppData\Local\clink
log              : C:\Users\arogl\AppData\Local\clink\clink.log
default settings : C:\Program Files (x86)\clink\default_settings
settings         : C:\Users\arogl\AppData\Local\clink\clink_settings
history          : C:\Users\arogl\AppData\Local\clink\clink_history
scripts          : C:\Program Files (x86)\clink ; C:\Users\arogl\AppData\Local\clink
default_inputrc  : C:\Program Files (x86)\clink\default_inputrc
inputrc          : %clink_inputrc%
                     (unset)
                 : state directory
                     C:\Users\arogl\AppData\Local\clink\.inputrc   (LOAD)
                     C:\Users\arogl\AppData\Local\clink\_inputrc
                 : %userprofile%
                     C:\Users\arogl\.inputrc
                     C:\Users\arogl\_inputrc
                 : %localappdata%
                     C:\Users\arogl\AppData\Local\.inputrc
                     C:\Users\arogl\AppData\Local\_inputrc
                 : %appdata%
                     C:\Users\arogl\AppData\Roaming\.inputrc
                     C:\Users\arogl\AppData\Roaming\_inputrc
                 : %home%
                     C:\Users\arogl\.inputrc
                     C:\Users\arogl\_inputrc
codepage         : 1252
keyboard langid  : 3081
keyboard layout  : 00000409

clink_arogl.log

clink_settings_arogl.txt

@chrisant996
Copy link
Owner

Thanks, everyone -- I found the issue. I'll publish a fix this evening.

In 2018 in commit 589b949 Martin accidentally introduced a buffer-splitting bug at 384 characters. It can split escape codes or even UTF8 characters. This has very likely been the source of a few weird intermittent display problems over the years.

The issue was specifically here.

(This also revealed that indeed some terminal implementations behave strangely when given incomplete escape sequences. But the primary root cause was in Clink, not in the terminal implementations.)

@rokrupnik @arogl Thank you VERY MUCH for the log files. I was able to gain many clues from them, including the fact that the output string was getting split somewhere between 360 and 396 characters (which is such an unusual number for a buffer size). Once I found the line of code with the 384 threshold, all the puzzle pieces fell into place.

This has been an issue for a very long time, but until now it was only happening in rare and intermittent cases.

It's very strange that even when I set my terminal width to a size that splits the buffer, the problem doesn't happen for me. I wonder if it may be due to the fact that I have the latest version of Windows Terminal installed, and maybe it has a fix for split hyperlink escape codes. I'm going to play a bit more and try to come up with a repro inside Clink, and also outside Clink, so I can be sure that the fix indeed solves the problems.

@chrisant996
Copy link
Owner

chrisant996 commented Nov 28, 2023

Screenshot shows the double prompt and then on windows resize it goes back to a single prompt until the next command is entered

@arogl Yes. However, that issue is completely unrelated:

  • ce973bf fixes the prompt moving up 1 line too far, plus not erasing all the text below the prompt (regression introduced in v1.5.17).
  • 396078a fixes the split escape code issue that was garbling the display (regression introduced in v1.5.14, but really just spotlighted a bug that's existed since Martin's initial v1.0 rewrite of Clink, but previously had only happened in rare cases).

@chrisant996
Copy link
Owner

chrisant996 commented Nov 28, 2023

Now that I know the root cause, I am able to force the issue to happen in the IDEA IntelliJ embedded terminal.

But I am completely unable to make it repro in Windows Terminal or in a legacy conhost terminal window, nor even in any version of Cmder from 1.3.18 through 1.3.24.

At this point, I'm pretty sure the reason I can't reproduce it in WT or conhost is because I have a version of WT installed that has a newer version of the conhost libraries than stock Win10 has, and the newer conhost library has a fix for handling split escape sequences. (Which would also explain why I can't repro in Cmder, since ConEmu uses the ConPty support when available.)

This completes the end-to-end analysis, and satisfies me that the root cause has been identified accurately and the fix in Clink v1.5.18 correctly addresses the issue.

@arogl
Copy link

arogl commented Nov 28, 2023

@chrisant996 I can confirm the update has fixed the double prompt on my version of WT preview.

@rokrupnik
Copy link
Author

@chrisant996 same here, I confirm the fix resolved the issue.

Thanks for the super quick fix! Also big THANK YOU for all the hard work you and the cmder team is doing to keep us sane in corporate Windows environments with super strict policies 😄 The least we can do is to put ourselves in your shoes at least for a minute and help you to help us!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants