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

removing and adding a new char in command causes new line inside cmd.exe #356

Closed
MagicAndre1981 opened this issue Sep 29, 2022 · 34 comments
Closed
Labels
bug Something isn't working

Comments

@MagicAndre1981
Copy link

Removing a character and adding a new char in command causes new line inside cmd.exe:

  • press UP arrow key to get last command
  • use left arrow key to navigate inside the command
  • press backspace to remove 1 char
  • now try to add a new char and for each new char you get a new line on screen, but if you press ENTER it runs the command fine

image

This looks to be a new issue since one of the last versions.

@chrisant996 chrisant996 added the bug Something isn't working label Sep 30, 2022
@chrisant996
Copy link
Owner

I'm not able to reproduce it following those steps.

There must be more to it. Can you help me with additional info?

  • What codepage is in use? Running chcp should say what codepage.
  • What version of Windows is in use?
  • Are you using ANSICON?
  • Are you using the default console host in Windows, or are you using a custom one, or are you using any other utilities to enhance how the console/terminal works?
  • When you use LEFT arrow key, specifically where does the cursor end up?
  • Does the BACKSPACE need to wrap around from the left edge to the right edge?
  • It looks like the input line wraps and takes multiple lines -- does the BACKSPACE need to happen on the second line, or on the first line?

And any other ideas you may have about what might influence the issue.

Yes, this is new in v1.3.43, as a side effect of having completely rewritten the display code, because of several problems in the Readline library's built in display code. I tried to test it thoroughly, but there may be some special cases that I missed.

@MagicAndre1981
Copy link
Author

As you see in the top this cmd picture is from Windows 8.1 with the normal cmd.exe

C:\Users\André>chcp
Aktive Codepage: 850.

The issue happens after linebreak, so moving cursor from 2nd to 1st line.

@chrisant996
Copy link
Owner

The repro steps look straightforward enough. But I can't reproduce it on Win10. So maybe it's somehow an issue with Win8.1's ANSI support.

As far as I can tell, Clink v1.3.43+ with the display rewrite should produce the same console output as v1.3.42 and earlier which used Readline's display routines. But clearly I am missing something, and there is some unexpected difference. Oh...or, I just realized, this issue might already exist in Readline on Win8.1, and Readline's implementation might merely trigger it much more rarely -- I did make a change to the math that decides which optimized output style to use.

I'll try to see if I can find a difference. But it turns out that I'm unable to find a difference, then @MagicAndre1981 could I ask you to do some diagnostic steps and send me the results?

(The main differences between the Readline display implementation versus the Clink custom rewrite should only be in edge cases where Readline's implementation has complex arithmetic errors. The scenario here does not intersect with any of those edge cases. And also Readline emits tons of separate outputs, but Clink emits a single complete output, which hugely improves the behavior when resizing the terminal width.)

@MagicAndre1981
Copy link
Author

I also see this when I press several times the UP key to get latest commands. In 8.1, there is no easy resizing of the terminal width like in Windows 10.

Add some debug code and I can give you the log

@chrisant996
Copy link
Owner

chrisant996 commented Sep 30, 2022

@MagicAndre1981 can you send two separate logs:

Run clink set debug.log_terminal 1 to enable detailed input/output logging.

Then, capture two logs:

  1. Using Clink v1.3.44:
    • Start Clink and make it cause an extra newline TWO times.
    • Copy/attach the clink.log file here. It will get overwritten by the next steps, so be sure to copy/attach it before continuing.
  2. Using Clink v1.3.42:
    • Do the same thing as above, but with the older Clink where the problem doesn't happen.
    • Please use exactly the same key presses as above, so that I can compare the two logs.

Special Notes:

  • Please make 2 extra newlines happen each time -- doing more than that will make it harder to compare the two logs.
  • Please use exactly the same key presses and number of key presses for both cases, so that the logs can be compared. If the key presses are different at all, it will make analysis more difficult and less reliable.

Thank you! And sorry for the annoying bug. 😜

@dogancelik
Copy link

dogancelik commented Oct 2, 2022

I have the same issue, I reverted to v1.4.42 for now.

I tried to log both versions here.

I use ConEmu v220308.

@chrisant996
Copy link
Owner

I have the same issue, I reverted to v1.4.42 for now.

I tried to log both versions here.

I use ConEmu v220308.

@dogancelik Wow, and you're using Win 10, even. I wonder why I can't reproduce the problem. I'll analyze the logs in the next couple of days and see what I can find.

@chrisant996
Copy link
Owner

@dogancelik would you be able to share two things?

  1. A copy of your prompt filter Lua script.
  2. The output from clink set.

These should help me understand why the differences in the logs are happening.

@dogancelik
Copy link

I don't have any custom prompt filters like git, but I am using clink-completions.

This is my prompt:

$ set PROMPT
PROMPT=$E[m$E[32m$E]9;8;"USERNAME"$E\@$E]9;8;"COMPUTERNAME"$E\$S$E[92m$P$E[90m$_$E[90m$$$E[m$S$E]9;12$E\

Clink set output

@chrisant996
Copy link
Owner

chrisant996 commented Oct 2, 2022

No success yet: I'm using ConEmu, size 172x45, Clink v1.3.44, with the same prompt string. I paste in the exact same 4 lines of text from the log you shared, and I move to the exact same cursor position, and press Ctrl-W 3 times. But the display looks correct for me.

There is some elusive difference somewhere... :(

@dogancelik Can you share your .inputrc file, if you have one?
Can you share your ConEmu.xml file from the ConEmu directory, or your user-ConEmu.xml from the Cmder\config directory if you're using Cmder?
Also, are you using ANSICON?

UPDATE: The uploaded logs had somehow gotten altered; all CR (0x0d) bytes got converted into LF (0x0a) bytes (it's clear they are not what was originally written out by Clink). Once I figured that out and fixed the logs, replaying both of the logs produces the expected output. Replaying the logs in plain conhost, in ConEmu, and in Windows Terminal all look fine. Both of us are using Windows 10 build 19043.

@dogancelik
Copy link

dogancelik commented Oct 2, 2022

@chrisant996 No I am not using ANSICON. I uploaded the logs (correct line endings hopefully) and ConEmu, inputrc settings here: removed

@chrisant996
Copy link
Owner

Yes, thanks, the logs have correct line endings now. Unfortunately, still no repro, even after doing all of these:

  • created a fresh profile
  • using a fresh install of clink with no Lua scripts
  • replaced my conemu.xml with yours
  • replaced my clink_settings with yours
  • replaced my .inputrc with yours

I'm analyzing the logs more deeply to see what differences exist in the output sequences. The new display routines have three specific differences in what they output, but none of those three should be able to cause anything like this (1. in a couple cases they emit CR CR instead of CR but that is just a minor inefficiency that I'll clean up soon, 2. they emit ESC [ col G to jump to column col instead of emitting + col ESC [ C to move right col columns, 3. they emit the entire output as a single string, instead of emitting tons of tiny strings like Readline does, which makes Readline slower).

So, I anticipate/hope to find a fourth difference of some kind in the logged output, which will hopefully be a clue to point me in the right direction to figure this out.

Otherwise, maybe it is related to codepage/locale??

@dogancelik What does chcp report for you?

@dogancelik
Copy link

dogancelik commented Oct 3, 2022

@chrisant996 I use chcp 65001.

In my test, this issue does not happen in CMD.exe (no ConEmu) when I manually clink inject.

This bug also does not happen on a fresh install of ConEmu v22.08.07

Edit: I think I found the issue, this bug happens in a fresh install of ConEmu v22.03.08

@MagicAndre1981's problem happens in CMD.exe without ConEmu so the problem may still exist on Windows 11 8's original Command Prompt.

@chrisant996
Copy link
Owner

@dogancelik Yes, thanks for helping track it down. What you experienced is Maximus5/ConEmu#2429 which was a regression in ConEmu. The reason it didn't affect Clink before v1.3.43 is that I replaced the Readline display routines with a more efficient custom implementation. Writing the entire output in a single more efficient API call exposes the ConEmu regression.

@MagicAndre1981 is experiencing a similar issue in the Windows 8.1 built-in conhost. I'll try to find a Win 8.1 machine somewhere. I bet that the more efficient output is exposing the old line wrapping bug in Windows that got fixed in Win 10 (there was some discussion about it in the ConEmu and Windows Terminal repos a while back, and I'd forgotten about that). I may be able to work around it by simply disabling the new efficiency smarts.

Re: Win 10 and 11 -- I use both of those daily, and both are working fine for both the default console and Windows Terminal.

@MagicAndre1981
Copy link
Author

@MagicAndre1981 is experiencing a similar issue in the Windows 8.1 built-in conhost. I'll try to find a Win 8.1 machine somewhere.

get it from the link and install a VM.

I'll try to capture now the logs

@MagicAndre1981
Copy link
Author

I removed some things before to only show the same actions:

clink_142.log
clink_144.log

@chrisant996
Copy link
Owner

I removed some things before to only show the same actions:

clink_142.log clink_144.log

Yay! I see the difference in the output. And indeed the Win8.1 console behaves differently than the Win10+ console for line wrapping. It is indeed the old line wrapping bug that was fixed in Win10+ as part of the Windows Terminal effort (the issue is whether the cursor wraps immediately when it reaches the right edge, or if wrapping is deferred until the next output that is printed).

I see a few options for how I can fix this.

Thank you, @MagicAndre1981 for reporting this and for sharing logs!

chrisant996 added a commit that referenced this issue Oct 4, 2022
I missed some edge cases when fixing #356.
@chrisant996
Copy link
Owner

Oops, I found some edge cases I missed, and they caused regressions on Win 10 and Win 11.

They should all be fixed now.

@MagicAndre1981
Copy link
Author

I'll test it when new version is released

@chrisant996
Copy link
Owner

@MagicAndre1981 v1.3.45 is published.

@MagicAndre1981
Copy link
Author

@MagicAndre1981 v1.3.45 is published.

ok, when my new line issue is gone, but when I press UP key, the last command is now corrupted. Also copying text is not correctly pasted. But this only happens some times, not always.

@chrisant996
Copy link
Owner

chrisant996 commented Oct 4, 2022

@MagicAndre1981 Can you share three things:

  • Your clink_history file. For privacy, you can email it to the address in my github profile. I delete such things as soon as I finish analyzing them.
    • Or, you can narrow it down to a subset of your clink_history file, and just share an excerpt that reproduces the issue.
  • The terminal width in columns.
  • A screen shot of the prompt.

Then I can use those to reproduce what you're seeing.

I used Up through my entire command history on both Win 10 and Win 8.1 (5431 history entries) and experienced no issues. So there is likely a specific edge case that you're hitting, which is not present in my command history.

UPDATE: I think I've found the cases you were hitting.
Please try this build:
clink.1.3.46.d6fdd0.zip
If the build still has problems, then please share the three things noted above so that I can track down what's happening.

chrisant996 added a commit that referenced this issue Oct 5, 2022
@MagicAndre1981
Copy link
Author

UPDATE: I think I've found the cases you were hitting. Please try this build: clink.1.3.46.d6fdd0.zip

the .46 work fine now

@chrisant996
Copy link
Owner

Great, thanks for confirming. v1.3.46 is published now with the fix (and also a fix for right-side prompts on Win8.1, which worked before).

@mcoret
Copy link

mcoret commented Oct 14, 2022

I think I have an issue, related to the discussed one. I have Windows 11, and whenever I'm searching through the history matches, longer lines from the history stay on the command line, only partially replaced with shorter history matches. This often leads to unreadable results, shown on the command line. I'm on the latest release.

@chrisant996
Copy link
Owner

chrisant996 commented Oct 15, 2022

I think I have an issue, related to the discussed one. I have Windows 11, and whenever I'm searching through the history matches, longer lines from the history stay on the command line, only partially replaced with shorter history matches. This often leads to unreadable results, shown on the command line. I'm on the latest release.

@mcoret Are you using ConEmu? Make sure you are using the latest ConEmu. There was a recent breakage in ConEmu that can cause that.

@mcoret
Copy link

mcoret commented Oct 15, 2022

I am using Windows Terminal and CMD, and both of them have the same issue. I've tried to remove all lua scripts, and the issue exists even on bare clink install. Also chcp 65001.

@chrisant996
Copy link
Owner

chrisant996 commented Oct 15, 2022

@mcoret ok that's interesting. I'm not able to reproduce anything like that.

Can you please share all of the following information? You can attach them here or email them to the address in my github profile.

  • Your clink_history file.
  • Your clink_settings file.
  • The output from clink info.
  • What is your prompt string? Or does the problem occur even with the default $p$g prompt string?
  • Specific steps -- e.g. type blah then press Up about 20 times until the blah asaksjdhfkljhasdlkfjhaslkdjflasjdhf history line shows up.

Thanks!

@mcoret
Copy link

mcoret commented Oct 15, 2022

Here are the files. I've redacted the username in the info, but apart from that everything is unchanged.
I have the default prompt string $p$g. I can reproduce the issue by typing g and then pressing Up 4 times, then by pressing Down the command line isn't clearing up completely.
Thank you!

clink.zip

@chrisant996
Copy link
Owner

@mcoret Thank you for the files -- I am able to reproduce the problem! Will track it down and get a fix out ASAP.

chrisant996 added a commit that referenced this issue Oct 15, 2022
The condition for when to clear leftover display content had an
inaccurate optimization.
@chrisant996
Copy link
Owner

To hit the bug, it was necessary to have something like this:

Before:

whatever> aaa_whatever_this_part_does_not_matter_aaa IDENTICAL DIFFERENT
something that wrapped

After:

whatever> bbb_whatever_this_part_does_not_matter_bbb IDENTICAL

And then the optimization had a slightly wrong condition, about whether the end of the display needed to be cleared.

Your history file contains cases of that. Apparently my own 5400+ entry history file does not. ¯_(ツ)_/¯

Thanks for catching and reporting this, @mcoret !

@mcoret
Copy link

mcoret commented Oct 15, 2022

all is fine! thank you for your great work!

@mcoret
Copy link

mcoret commented Oct 18, 2022

Hi!
Can you please check the following history file?
I'm experiencing a similar problem when pressing a (or x) and then, after pressing Up three times and then Down, the command line isn't clearing up completely. Note there is a trailing space character in some of the fields.
Thank you!

clink_history.txt

@chrisant996
Copy link
Owner

@mcoret, thank you for the excellent repro steps. a then Up x3 is enough to hit the issue. Pressing Down after that hits the issue again.

The trailing space is indeed the trigger. Another misguided optimization. Sorry about that!

Thanks for reporting it; I'll publish an update shortly.

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

4 participants