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.

Sign up for GitHub

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

Jump to bottom

FinalTerm escape sequences are stripped from prompt filter #457

Closed
krzysdz opened this issue May 11, 2023 · 5 comments
Closed

FinalTerm escape sequences are stripped from prompt filter #457

krzysdz opened this issue May 11, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@krzysdz
Copy link

krzysdz commented May 11, 2023

I'm trying to add shell integration using FinalTerm escape sequences following the OSC 133 ; <something> ST format, but all these sequences from filter are removed before printing to the terminal.

ecma48_code::decode_osc decodes these sequences as { .command = '1', .subcommand = '0', .visible = false } and all !visible OSC codes with the command property between 0 and 2 (inclusive) are treated as set console title commands and skipped from the output:

clink/clink/terminal/src/ecma48_iter.cpp

Lines 560 to 571 in 17b5a3e

if (osc.visible)
{
if (out)
out->concat(osc.output.c_str(), osc.output.length());
if (cell_count)
cells += clink_wcwidth(osc.output.c_str(), osc.output.length());
}
else if (!apply_title)
goto concat_verbatim;
else if (osc.command >= '0' && osc.command <= '2')
set_console_title(osc.param.c_str());
else

A simple example that demonstrates the problem:

local p = clink.promptfilter(10)
function p:filter(prompt)
    -- OSC 133 sequences from prompt filter are removed before writing to the terminal
    return "\001\x1b]133;D;"..os.geterrorlevel().."\007\002\001\x1b]133;A\007\002"..prompt.." \001\x1b]133;B\007\002"
end

local function command_executed_mark(input)
    -- this works
    clink.print("\001\x1b]133;C\007\002", NONL)
end

clink.onendedit(command_executed_mark)
@chrisant996
Copy link
Owner

If they're stripped, then it means Clink doesn't recognize the terminal program, and was forced to fall back to its own built-in terminal emulator.

See the terminal.emulation setting.

@krzysdz
Copy link
Author

krzysdz commented May 11, 2023

I was testing it with Windows Terminal Dev (microsoft/terminal@48eee4d, build from CI) with terminal.emulation = native and now tested with emulate too, but it didn't change anything.

@chrisant996
Copy link
Owner

I see, yes. I'll change how it parses OSC codes so it doesn't assume a single digit anymore.

Also, the sample Lua script has problems -- it is important to remove the \001 and \002 sequences from the Lua script:

  1. Clink automatically adds \001 and \002 characters where needed for the Readline library. If you manually add them as well, then you're doubling them up as \001\001 and \002\002, and the Readline library can get confused about the width of the prompt and whether/where it wraps to a new line, which can cause garbled output while typing input at the command promtp.
  2. In the clink.print() case they are literally printed to the terminal (the Readline library is not involved, so they go straight to the terminal without being stripped).

The \001 and \002 stuff is specific to Readline, and not to anything else. Clink automatically handles adding them; you shouldn't add them yourself.

@chrisant996 chrisant996 added the bug Something isn't working label May 11, 2023
@krzysdz
Copy link
Author

krzysdz commented May 11, 2023

Thank you and thanks for the remarks regarding \001 and \002!

@chrisant996
Copy link
Owner

It was simpler than I anticipated: unrecognized OSC codes were accidentally reported as though they were recognized. But the code that tried to handled recognized OSC codes didn't know what to do with them, so they got accidentally stripped. The fix is simply to accurately report whether OSC codes are recognized.

@krzysdz krzysdz mentioned this issue May 13, 2023
Merged
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@krzysdz @chrisant996
-