cmderdev / cmder

Lovely console emulator package for Windows
https://cmder.app
MIT License
25.88k stars 2.03k forks source link

[Bug] Reserve-i-search return broken string #2821

Closed trungklam closed 1 year ago

trungklam commented 1 year ago

Version Information

Cmder version:Version 1.3.21
Operating system: Windows10

Cmder Edition

Cmder Mini

Description of the issue

When I use reserve-i-search (Ctrl+R) to cycle throw the history, press the right arrow button to complete it ("->") return me broken text instead of the full command.

  1. In search image
  2. After completion image

How to reproduce

  1. Use reserve-i-search (Ctrl+R) to search for history
  2. Completing it by right arrow key (->)

Additional context

Possibly related to #2705

Checklist

trungklam commented 1 year ago

Hi @chrisant996 , sorry for bother you but is this somehow related to clink/clink-completion ? I tested it with both bundle version of clink and also latest clink version, the issue still happen

chrisant996 commented 1 year ago

Issue 2705 is about ssh, and appears to be using the bash shell, so it's probably not related to what you're experiencing.

I'm not able to reproduce the behavior you've described.

Can you share these, please?

Also, can you clarify what is meant by "press the right arrow button to complete it"? I want to understand if there is an expectation of some kind of special completion behavior, or if "complete it" just means cancel search mode.

If you can figure out what specific things are necessary in order for the problem to occur, then I'll be able to make it happen and investigate.

Here is another idea that might help (or might not): You could run clink set debug.log_terminal true, then make the problem occur, and then send the clink.log file to the email address in my github profile.

trungklam commented 1 year ago

Hi @chrisant996 ,here are the one you asked, I hope it can help

clink set autosuggest.async True autosuggest.enable True autosuggest.original_case True autosuggest.strategy match_prev_cmd history completion clink.autostart clink.autoupdate False clink.colorize_input True clink.default_bindings bash clink.logo full clink.max_input_rows 0 clink.paste_crlf crlf clink.path clink.promptfilter True clink.update_interval 5 cmd.admin_title_prefix cmd.altf4_exits True cmd.auto_answer off cmd.ctrld_exits True cmd.get_errorlevel True color.arg bold color.arginfo yellow color.argmatcher color.cmd bold color.cmdredir bold color.cmdsep bold color.comment_row bright white on cyan color.description bright cyan color.doskey bold cyan color.executable color.filtered bold color.flag default color.git.star bright green color.hidden color.histexpand color.horizscroll color.input color.interact bold color.message default color.modmark color.popup color.popup_desc color.prompt color.readonly color.selected_completion color.selection color.suggestion bright black color.unexpected default color.unrecognized color.yarn.module bright green color.yarn.script bright blue debug.log_terminal False directories.dupe_mode add doskey.enhanced True exec.aliases True exec.commands True exec.cwd True exec.dirs True exec.enable True exec.files False exec.path True exec.space_prefix True files.hidden True files.system False history.auto_expand True history.dont_add_to_history_cmds exit history history.dupe_mode erase_prev history.expand_mode not_dquoted history.ignore_space False history.max_lines 10000 history.save True history.shared True history.sticky_search False history.time_format %F %T history.time_stamp off lua.break_on_error False lua.break_on_traceback False lua.debug False lua.path lua.reload_scripts False lua.strict True lua.traceback_on_error False match.expand_abbrev True match.expand_envvars False match.fit_columns True match.ignore_accent True match.ignore_case relaxed match.max_fitted_matches 0 match.max_rows 0 match.preview_rows 5 match.sort_dirs with match.substring False match.translate_slashes system match.wild True prompt.async True prompt.transient off readline.hide_stderr False terminal.adjust_cursor_style True terminal.differentiate_keys False terminal.east_asian_ambiguous auto terminal.emulation auto terminal.mouse_input auto terminal.mouse_modifier terminal.raw_esc False terminal.use_altgr_substitute False

.inputrc version : 1.4.15.439299 session : 1328 binaries : C:\Users\happiness\Development\cmder\vendor\clink state : C:\Users\happiness\Development\cmder\config log : C:\Users\happiness\Development\cmder\config\clink.log settings : C:\Users\happiness\Development\cmder\config\clink_settings history : C:\Users\happiness\Development\cmder\config\clink_history scripts : C:\Users\happiness\Development\cmder\vendor ; C:\Users\happiness\Development\cmder\vendor\clink ; C:\Users\happiness\Development\cmder\config inputrc : %clink_inputrc% (unset) : state directory C:\Users\happiness\Development\cmder\config.inputrc (LOAD) C:\Users\happiness\Development\cmder\config_inputrc : %userprofile% C:\Users\happiness.inputrc C:\Users\happiness_inputrc : %localappdata% C:\Users\happiness\AppData\Local.inputrc C:\Users\happiness\AppData\Local_inputrc : %appdata% C:\Users\happiness\AppData\Roaming.inputrc C:\Users\happiness\AppData\Roaming_inputrc : %home% C:\Users\happiness.inputrc C:\Users\happiness_inputrc

and the clink.log when issue happen clink.log

For "press the right arrow button to complete it", yes I normally press the right arrow button on my keyboard to complete the search - mean returning the command line found in the history and escape the current search image

chrisant996 commented 1 year ago

@Happin3ss thanks for the diagnostic information -- the clink.log file showed what is happening.

The prompt string is incorrect.

What are you using to generate your prompt string?

It ends with ESC [ K ESC [ 0 J, and that's causing the problem:

The problem also happens in bash and Readline

If you use the same prompt string in bash, use Ctrl-R to search and find a history line that wraps onto 2 or more lines, and press Right, then the entire input line disappears.

So, those extra characters in the prompt are incorrect to have in a prompt string.

Why does it happen in Clink now, but not before?

Previously, it happened in Clink under the same circumstances where it happens in bash (such as the previous example).

But after I made Clink stop using the Readline display routines, and wrote custom display routines, the new routines are more efficient and more predictable. And they realize that they can simply shift some existing characters left, instead of reprinting stuff manually. But by that time, the prompt string has already wiped out the input line display, and so there's nothing to shift over.

Conclusion

This is caused by an incorrect prompt string.

However, I would bet that there are a lot of incorrect prompt strings out there, because quirks in Readline's display routines make it less common for the incorrectness to become noticeable. Quick "sanity testing" would easily lead someone to assume that it's valid to include those escape codes in a prompt string. But it's not, and never has been.

But I'll make Clink smart enough to compensate for that kind of incorrectness

Technically the problem is caused by user error. But, often a user will simply have used someone else's prompt generator program or prompt string. And in that case it is some other user's error, causing a problem for the current user. And that can feel very frustrating, and it can make it very difficult for the current user to fix the mistake without losing their custom prompt configuration.

I'll make Clink smart enough to automatically compensate when those escape codes are in a prompt string. And I'll make the Ctrl-X,Ctrl-Z diagnostic command smart enough to detect the incorrectness and show an alert about it.

chrisant996 commented 1 year ago

@Happin3ss Clink v1.4.16 will do its best to compensate for these kind of mistakes in prompt strings. And the Ctrl-X,Ctrl-Z diagnostics command will also include analysis of the prompt string, looking for problematic codes.

However, the best solution is to fix the generated prompt string. It's malformed, and will have problems in every shell, under various conditions.

chrisant996 commented 1 year ago

P.S. @DRSDavidSoft re: the label on this issue; it isn't a bug in Cmder or ConEmu or Clink, it's user error or a bug in a prompt generator program (e.g. oh-my-posh or starship), or most likely a bug in a specific theme for a prompt generator program (probably several themes are malformed in this way).

DRSDavidSoft commented 1 year ago

@chrisant996 Wow, I was amazed by your through examination and the compensation for the user error in prompt. Thanks for trying to address this, I'll adjust the labels!

trungklam commented 1 year ago

@chrisant996 thanks a lot for your clarification. Just quick question is there anyway I can fix this manually (e.g delete those symbols in history file). I tried to look at the clink_history but it was not there. I'm still not so sure how to prevent this from happen again. For now I'm using Cmder/Powershell in Windows Terminal with oh-my-posh installed. Probably oh-my-posh is the root cause but If that's the case then I guess should be more people ran into this trouble.

chrisant996 commented 1 year ago

@chrisant996 thanks a lot for your clarification. Just quick question is there anyway I can fix this manually (e.g delete those symbols in history file). I tried to look at the clink_history but it was not there.

@Happin3ss This has nothing at all to do with history.

This is only about the prompt string.

The crucial question might have been missed, so I'll ask it again:

What are you using to generate your prompt string?

trungklam commented 1 year ago

@chrisant996 thanks a lot for your clarification. Just quick question is there anyway I can fix this manually (e.g delete those symbols in history file). I tried to look at the clink_history but it was not there.

@Happin3ss This has nothing at all to do with history.

This is only about the prompt string.

The crucial question might have been missed, so I'll ask it again:

What are you using to generate your prompt string?

Hi @chrisant996 I'm not so sure honestly. I'm using a default cmder setup (with clink and clink completion bundled by default), theming with oh-my-posh. So I believe it should be clink, with history-search-backward and history-search-forward

I can confirm Clink 1.4.16 make it works again . Thank you a lot ! I'm also open to assist to find out the root cause of this issue, to prevent it happen in future for other users.

chrisant996 commented 1 year ago

Hi @chrisant996 I'm not so sure honestly. I'm using a default cmder setup (with clink and clink completion bundled by default), theming with oh-my-posh. So I believe it should be clink, with history-search-backward and history-search-forward

Ok, you're using oh-my-posh.

I still don't know which theme, though.

I've tried to give the short version of explaining precisely what is wrong in the prompt string.

And I also explained how to reproduce the exact same problem in bash. The steps are a little different, but that's because of ancient quirks in bash that are meant for optimizing the amount of data that would get sent over an old 1200 baud modem. Those quirks cause bash to use different drawing strategies at different times.

I can understand that you might not have familiarity with the details of prompt strings and escape codes and how prompts and input lines get drawn by programs. I can understand if you don't believe me or my expertise or detailed analysis.

But that doesn't change that the problem is in the theme you're using.

In fact, if you try a bunch of other themes, you'll probably find that most of them don't have the problem.

I can confirm Clink 1.4.16 make it works again . Thank you a lot ! I'm also open to assist to find out the root cause of this issue, to prevent it happen in future for other users.

The only thing left is for you to share which theme you're using with oh-my-posh. The bug is in the theme.

Then I can report it to Jan and the theme author, on your behalf. They'll understand and recognize why it's a problem, and they'll know the right steps to take to fix the theme bug (the same steps I described earlier).

The bug was not in Clink. The change I made in Clink detects the theme bug, and TURNS OFF OPTIMIZED OUTPUT. Clink goes into defensive mode and becomes super inefficient, to compensate for the theme bug. The reason the precisely same cited steps don't reproduce the problem in bash is because bash accidentally falls into an unoptimized mode. I'm leaving out a lot of specific technical details about the algorithm and escape codes chosen by bash.

Anyway, the ONLY reason I changed Clink was because most people don't understand enough about escape codes to recognize why the problem is happening or who to report it to. I'm mainly trying to reduce the support cost that I have to pay for someone else's bugs. 🙃

trungklam commented 1 year ago

HI @chrisant996 ,sorry if I've made you feel that I dont trust in your explanation. In fact, it is one of the most detail and friendly one I've got with an issue report 😄 Here is the theme I'm using, I believe it's a custom one (not sure where I got it, quite long ago - from a medium post probably). agnoster-custom.omp.json.zip I will change to use a default theme provided by oh-my-posh, not a big deal for me. Thank you again for making clink be able to fix an issue that was not causing by it 😄

chrisant996 commented 1 year ago

It looks like oh-my-posh always appends ESC[K ESC[0J.

There are comments in its source code saying it needs to do that in PowerShell due to a bug in some case. I can't see yet what is making it also show up for all the other shells as well.

I started an email conversation with Jan and shared the info about bash, Readline, Clink, etc.

Anyway, Clink v1.4.16 and newer completely compensate for the ESC[K ESC[0J.

But you should be aware that other shells will likely have edge cases where the ESC[K ESC[0J cause the input line display to become corrupted, depending on the specific text and operations that are used.

chrisant996 commented 1 year ago

@DRSDavidSoft this isn't related to Cmder. This is an issue between oh-my-posh and various shells.

It's debatable whether the shells should be smarter, or whether oh-my-posh should be more conservatively compatible. I don't have time to get sucked into debates like that, and so when it's easy to compensate then I just make a change in code that I control. 🙃 (Which I did, in Clink v1.4.16)

chrisant996 commented 1 year ago

Jan made a fix in oh-my-posh.

The problem is reproducible in bash on Linux (as well as other shells and platforms, etc).

chrisant996 commented 1 year ago

This can be closed.