Fix Windows width calculation

[sophiajt]

This fixes the Windows prompt width calculation by using the same width calculation Unix uses. This code might have originally been in place to work around a Windows shortcoming, but it seems that current versions of Windows calculate width the same way Unix does

Before:

After:

[gwenn]

Sorry, I don't have access to a Windows machine right now (and only Windows 7 at work). I guess your PR is fine but I don't understand why! Because there should be no ansi escape sequence in prompt (only highlight_prompt should inject esc. seq.). On unix platform, to keep backward compatibility, you can still use esc. seq. directly in prompt (but should not).

[sophiajt]

Would the above fix not work for highlight_prompt?

[gwenn]

Your fix will work for highlight_prompt. But prompt is not supposed to contain escape sequence (at least) on windows. Could you please confirm that rustyline example was run without any modification ? Thanks.

[sophiajt]

Yes, the normal prompt works. This fix is for when people put ANSI sequences into their prompts.

[sophiajt]

Basically it lets both kinds of prompts work correctly.

[sophiajt]

oh I see what you're saying. You're talking about the screenshot. No, the PR doesn't change the default behavior of the example. I just changed it to make my testing a little easier.

[gwenn]

Ok, Your screenshots are the result of rustyline example with the following modifications: a prompt containing ansi escape sequences directly. Right ?

My concerns are that:

• ansi escape sequences are not supported yet on Windows < 10 => highlighting is disabled.
• highlighting/colouring is (should be) disabled when terminal is not supported.
• highlighting/colouring is (should be) disabled when stdout is not a tty.
• highlighting may be disabled manually (https://docs.rs/rustyline/4.0.0/rustyline/config/enum.ColorMode.html).

When prompt already contains ansi escape sequences, it's not easy/efficient to remove them (when highlighting is disabled).

So I am not sure:

• with your PR, behaviour is consistent on unix and Windows 10,
• but highlighting cannot be disabled dynamically anymore.

[sophiajt]

Just to be clear, the PR does not change the default prompt. The screenshot is only from my testing and is not part of the PR.

The PR only changes how the length is calculated. If there is no ANSI in the prompt, the length is calculated correctly.

[sophiajt]

@gwenn - I'm not clear what you're asking me to change so that the PR can be accepted.

[gwenn]

@jonathandturner I am so sorry. With ConEmu (and Windows 7), without your PR but this :

diff --git a/src/tty/windows.rs b/src/tty/windows.rs
index 2a44206..aa47bbd 100644
--- a/src/tty/windows.rs
+++ b/src/tty/windows.rs
@@ -550,6 +550,7 @@ impl Term for Console {
                 let raw = original_stdstream_mode | wincon::ENABLE_VIRTUAL_TERMINAL_PROCESSING;
                 self.ansi_colors_supported =
                     unsafe { consoleapi::SetConsoleMode(self.stdstream_handle, raw) != 0 };
+                self.ansi_colors_supported = true;
             }
             Some(original_stdstream_mode)
         } else {

The prompt is displayed correctly:

Could you please tell me:

• the Windows version,
• the console (cmd, powershell, conemu, ...)
• rustyline version/sha

?

And if you have time, without your PR, add this line:

diff --git a/src/tty/windows.rs b/src/tty/windows.rs
index 2a4420689..ca0feeccf 100644
--- a/src/tty/windows.rs
+++ b/src/tty/windows.rs
@@ -398,6 +398,7 @@ impl Renderer for ConsoleRenderer {
             pos.col = 0;
             pos.row += 1;
         }
+        debug!(target: "rustyline", "orig: {:?}, s: {:?}, pos: {:?}", orig, s, pos);
         pos
     }

And send me the logs. Thanks.

[sophiajt]

I'm on cmd in Windows 10 (latest stable release). Using git sha which was the latest as of a few days ago aba9b21e0261e8dd5145b315eaf255e35579a49a

The fix you suggested of forcing ansi doesn't fix the calculation.

Here are some sample logs:

     Running `target\debug\examples\example.exe`
[2019-05-15T12:46:27Z DEBUG rustyline::tty::windows] orig: Position { col: 0, row: 0 }, s: "\u{1b}[1;32m>>\u{1b}[0m ", pos: Position { col: 12, row: 0 }
[2019-05-15T12:46:27Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "", pos: Position { col: 12, row: 0 }
[2019-05-15T12:46:27Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "", pos: Position { col: 12, row: 0 }
>>          [2019-05-15T12:46:30Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 't')
[2019-05-15T12:46:30Z DEBUG rustyline::undo] Changeset::insert(0, 't')
[2019-05-15T12:46:30Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "t", pos: Position { col: 13, row: 0 }
t[2019-05-15T12:46:30Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 'e')
[2019-05-15T12:46:30Z DEBUG rustyline::undo] Changeset::insert(1, 'e')
[2019-05-15T12:46:30Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "te", pos: Position { col: 14, row: 0 }
e[2019-05-15T12:46:30Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 's')
[2019-05-15T12:46:30Z DEBUG rustyline::undo] Changeset::insert(2, 's')
[2019-05-15T12:46:30Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "tes", pos: Position { col: 15, row: 0 }
s[2019-05-15T12:46:30Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 't')
[2019-05-15T12:46:30Z DEBUG rustyline::undo] Changeset::insert(3, 't')
[2019-05-15T12:46:30Z DEBUG rustyline::tty::windows] orig: Position { col: 12, row: 0 }, s: "test", pos: Position { col: 16, row: 0 }

The column miscalculation is what this PR attempts to fix.

[gwenn]

Ok,

[2019-05-15T12:46:27Z DEBUG rustyline::tty::windows] orig: Position { col: 0, row: 0 }, s: "\u{1b}[1;32m>>\u{1b}[0m ", pos: Position { col: 12, row: 0 }

Is unexpected ! There should be no ansi escape sequence when calling calculate_position.

static PROMPT: &'static str = ">> ";
...
let readline = rl.readline(PROMPT);
...     let mut s = State::new(&mut stdout, prompt, helper, ctx);
...             let prompt_size = out.calculate_position(prompt, Position::default());

Expected:

[2019-05-15T16:26:05Z DEBUG rustyline::tty:: windows] orig: Position { col: 0, row: 0 }, s: ">> ", pos: Position { col: 3, row: 0 }

[sophiajt]

@gwenn - as I mentioned earlier I am toggling the prompt to be the ansi prompt to test both non-ansi and ansi prompts.

The non-ansi version works fine (but it's not what I'm trying to fix). Example:

    Finished dev [unoptimized + debuginfo] target(s) in 0.92s
     Running `target\debug\examples\example.exe`
[2019-05-15T16:31:30Z DEBUG rustyline::tty::windows] orig: Position { col: 0, row: 0 }, s: ">> ", pos: Position { col: 3, row: 0 }
[2019-05-15T16:31:30Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "", pos: Position { col: 3, row: 0 }
[2019-05-15T16:31:30Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "", pos: Position { col: 3, row: 0 }
>> [2019-05-15T16:31:31Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 'q')
[2019-05-15T16:31:31Z DEBUG rustyline::undo] Changeset::insert(0, 'q')
[2019-05-15T16:31:31Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "q", pos: Position { col: 4, row: 0 }
[2019-05-15T16:31:31Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "q", pos: Position { col: 4, row: 0 }
>> q[2019-05-15T16:31:33Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 'i')
[2019-05-15T16:31:33Z DEBUG rustyline::undo] Changeset::insert(1, 'i')
[2019-05-15T16:31:33Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "qi", pos: Position { col: 5, row: 0 }
[2019-05-15T16:31:33Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "qi", pos: Position { col: 5, row: 0 }
>> qi            

[sophiajt]

Are you saying that putting ansi in the default prompt is what is breaking it?

[sophiajt]

I see. Sorry it took so long. I couldn't understand why it wouldn't be okay to have ANSI codes in the prompt.

As best as I can figure out now, you put two versions of the prompt in there: one with colors and one without. The one without colors is the one used for the calculation.

I went back to using a simple prompt and a colored prompt separately, and then did .color_mode(ColorMode::Forced) to used the colored prompt. This appears to work correctly with your fix.

     Running `target\debug\examples\example.exe`
[2019-05-15T16:41:18Z DEBUG rustyline::tty::windows] orig: Position { col: 0, row: 0 }, s: ">> ", pos: Position { col: 3, row: 0 }
[2019-05-15T16:41:18Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "", pos: Position { col: 3, row: 0 }
[2019-05-15T16:41:18Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "", pos: Position { col: 3, row: 0 }
>> [2019-05-15T16:41:19Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 't')
[2019-05-15T16:41:19Z DEBUG rustyline::undo] Changeset::insert(0, 't')
[2019-05-15T16:41:19Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "t", pos: Position { col: 4, row: 0 }
[2019-05-15T16:41:19Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "t", pos: Position { col: 4, row: 0 }
>> t[2019-05-15T16:41:19Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 'h')
[2019-05-15T16:41:19Z DEBUG rustyline::undo] Changeset::insert(1, 'h')
[2019-05-15T16:41:19Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "th", pos: Position { col: 5, row: 0 }
[2019-05-15T16:41:19Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "th", pos: Position { col: 5, row: 0 }
>> th[2019-05-15T16:41:19Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 'i')
[2019-05-15T16:41:19Z DEBUG rustyline::undo] Changeset::insert(2, 'i')
[2019-05-15T16:41:19Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "thi", pos: Position { col: 6, row: 0 }
i[2019-05-15T16:41:20Z DEBUG rustyline::keymap] Emacs command: SelfInsert(1, 's')
[2019-05-15T16:41:20Z DEBUG rustyline::undo] Changeset::insert(3, 's')
[2019-05-15T16:41:20Z DEBUG rustyline::tty::windows] orig: Position { col: 3, row: 0 }, s: "this", pos: Position { col: 7, row: 0 }

[gwenn]

Yes, because there is no way to remove them on Windows < 10 where ansi esc seq are not supported, or if stdout is not a tty, or when the terminal is not supported.

And, I guess there is something wrong in rustyline because you should not have to force the color mode (on Windows 10). The bug should be here:

            // To enable ANSI colors (Windows 10 only):
            // https://docs.microsoft.com/en-us/windows/console/setconsolemode
            if original_stdstream_mode & wincon::ENABLE_VIRTUAL_TERMINAL_PROCESSING == 0 {
                let raw = original_stdstream_mode | wincon::ENABLE_VIRTUAL_TERMINAL_PROCESSING;
                self.ansi_colors_supported =
                    unsafe { consoleapi::SetConsoleMode(self.stdstream_handle, raw) != 0 };
            }

[gwenn]

I will try to have access to a windows 10 machine... https://azure.microsoft.com/en-us/pricing/details/virtual-desktop/ ?

[sophiajt]

Since this PR fixes the issue where people use ANSI in their prompts, wouldn't it be okay to just accept it? It doesn't break people who don't use ANSI.

A use case I've seen is something like this:

let readline = rl.readline(&format!(
            "{}> ",
            Color::Green.paint(filepath.to_string())
        ));

Where you're also mixing in parameters into the prompt to display things like:

C:\Source> 

The most natural place to put this is when you write out the prompt. I'm not sure how else you would do it.

[gwenn]

Something like this. I will try to see if I can fix the lifetime mismatch.

[gwenn]

But you are right. It's not user friendly. https://python-prompt-toolkit.readthedocs.io/en/master/pages/reference.html?highlight=prompt#module-prompt_toolkit.formatted_text

Many places in prompt_toolkit can take either plain text, or formatted text. For instance the prompt() function takes either plain text or formatted text for the prompt.

[gwenn]

Ok, here is an example with a non static prompt. Let me know if it looks awkward.

[gwenn]

Windows bug related to ansi colors is also fixed: #227.

[PaddiM8]

Any way around this? Is this going to be fixed? :/

[Luis-Weinzierl]

Will there finally be a fix?

[gwenn]

As already explained, there should be no ansi escape seq directly in the prompt, only highlight_prompt may return ansi escape seq. Or we need a PR to remove ansi escape seq if terminal doesn't support them...

[Luis-Weinzierl]

@gwenn You could also just allow the user to feed a usize into a readline function that places the cursor at the given distance from the beginning. That would allow for ansi codes to be used but doesn't affect non-ansi apps.

[gwenn]

@BlackBirdTV You mean that rustyline user should compute the size of the prompt ? And cursor may not be at the end of the prompt by default because you can have an initial input... Also final user may want no color but how to remove the ansi sequences from your already styled prompt ?

We may introduce a Prompt trait like:

trait Prompt {
  /// ANSI esc sequences stripped used colors are not supported or disabled.
  fn rawText(&self) -> &str;
  /// May contains ANSI esc sequences
  fn styledText(&self) -> &str;
  /// Display size
  //fn width(&self) -> usize;
}

with a default impl for &str. And your impl would need something like strip-ansi-escapes for rawText.