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

Incompatible ANSI escape sequence at the end #277

Closed
2 tasks done
rashil2000 opened this issue Dec 27, 2020 · 21 comments
Closed
2 tasks done

Incompatible ANSI escape sequence at the end #277

rashil2000 opened this issue Dec 27, 2020 · 21 comments
Labels
🐛 bug Something isn't working windows

Comments

@rashil2000
Copy link

rashil2000 commented Dec 27, 2020

Prerequisites

  • I have read and understand the CONTRIBUTING guide
  • I looked for duplicate issues before submitting this one

Description

While tinkering around with Clink, the author (@chrisant996) of the revived project discovered a bug in the default theme of OMP3. This does not cause any functional issues, but renders the theme incompatible with Readline libraries on many platforms.

The details of the discovery are mentioned here chrisant996/clink#39, while explanations and reproduction steps are mentioned in chrisant996/clink#40.

Environment

  • Oh my Posh version: 3.61.1
  • Theme: Default
  • Operating System: any
  • Shell: any
  • Terminal: any
@chrisant996
Copy link
Contributor

It causes a functional issue in Clink, and should cause exactly the same functional issue in Bash, since both use the GNU Readline library which is the component affected by the oh-my-posh issue.

The CSI K escape code near the end of the prompt string can, under certain circumstances, accidentally erase the input text from the screen. Readline doesn't expect anything else to be printing over its output, and for performance reasons it's important not to print over or erase Readline's output. But the CSI K escape code does.

The linked issue 40 has repro steps for one example scenario. Many other scenarios exist.

@JanDeDobbeleer
Copy link
Owner

JanDeDobbeleer commented Dec 27, 2020

@chrisant996 The clear until eol escape sequence is there because Powershell has a bug where it colors the line after the prompt when the prompt scrolls up when it's at the bottom of your console. So removing that will break Powershell, but it can be a Powershell only feature obviously.

About the speed, it's weird but people have been reporting it's faster than starship, much to my surprise, although I noticed Windows is noticeably slower for some unknown reason.

Also, care to clarify point 2 more specifically?

JanDeDobbeleer added a commit that referenced this issue Dec 27, 2020
@lnu
Copy link
Contributor

lnu commented Dec 27, 2020

We already discussed this in the past but what slows down(mostly) the prompt on windows is the git segment.
Maybe we could check what's done differently in starship on this point?
And generally, syscall are really slow in windows compared to mac or linux.

@JanDeDobbeleer
Copy link
Owner

the git segment

To be honest, we do A LOT more than others in regards to git functionality and I would like to keep it that way. I did create a workaround for the sys calls, which should have avoided that timeout as much as possible without having to fork go.

@JanDeDobbeleer JanDeDobbeleer added 🐛 bug Something isn't working windows labels Dec 27, 2020
@lnu
Copy link
Contributor

lnu commented Dec 27, 2020

the git segment

To be honest, we do A LOT more than others in regards to git functionality and I would like to keep it that way. I did create a workaround for the sys calls, which should have avoided that timeout as much as possible without having to fork go.

True, I checked starship and it shows less informations(at least by default) than omp.

@JanDeDobbeleer
Copy link
Owner

@lnu I'm going to create a POC to see if we can delegate the syscalls to a Rust lib. It's perfectly possible and it could solve the issue entirely. See this repo for example.

@chrisant996
Copy link
Contributor

@chrisant996 The clear until eol escape sequence is there because Powershell has a bug where it colors the line after the prompt when the prompt scrolls up when it's at the bottom of your console. So removing that will break Powershell, but it can be a Powershell only feature obviously.

Ah, mutually exclusive issues -- the most fun of all problems. 😄

Maybe some kind of callout in the documentation could even be sufficient. As you say, some way to influence the behavior based on what shell is targeted.

About the speed, it's weird but people have been reporting it's faster than starship, much to my surprise, although I noticed Windows is noticeably slower for some unknown reason.

I've only just discovered and installed both. I'll try some comparisons and share what I learn. I might also take a couple minutes to see if the Visual Studio profiler reveals any significant time consumers.

For me, oh-my-posh takes around 2 to 3 seconds to run on Windows when using the default theme. I'll post more specific details when I get a chance.

Also, care to clarify point 2 more specifically?

Sure. It generates an almost 2 KB string for around 80 characters of text. It seemed there are many series of color escape codes back to back, often redundant, with no text in between.

Perhaps something about my environment hits a case not normally seen. When I get a chance I'll upload a file containing the full string, and maybe annotate redundant or superfluous sequences.

@chrisant996
Copy link
Contributor

We already discussed this in the past but what slows down(mostly) the prompt on windows is the git segment.
Maybe we could check what's done differently in starship on this point?
And generally, syscall are really slow in windows compared to mac or linux.

If you can point me where to look in the oh-my-posh sources for the relevant git stuff I can compare it with what the Lua git prompt scripts for Clink do. Maybe that could reveal some interesting differences. The Lua scripts take maybe half a second for the git prompt info.

The oh-my-posh executable seems very large on Windows. It's over 8 MB, which suggests it may have a lot of framework code. Sometimes large frameworks hurt load and/or initialization performance. Just an observation; I don't have any data yet about where time is consumed.

@rashil2000
Copy link
Author

True, I checked starship and it shows less informations(at least by default) than omp.

I think this misses the point. Starship is, for most purposes, not a ready-to-go prompt utility; it shows very few things by default and needs a configuration file. I have uploaded my starship.toml here for reference.

@lnu
Copy link
Contributor

lnu commented Dec 27, 2020

True, I checked starship and it shows less informations(at least by default) than omp.

I think this misses the point. Starship is, for most purposes, not a ready-to-go prompt utility; it shows very few things by default and needs a configuration file. I have uploaded my starship.toml here for reference.

I did not take the time to configure it extensively. Thanks for your config.

@JanDeDobbeleer
Copy link
Owner

oh-my-posh takes around 2 to 3 seconds to run on Windows

@chrisant996 that's definitely not normal. It runs below a second on my machines (and also confirmed by others). The only time this occurred, it disappeared after a reboot of the user, and as far as I know hasn't returned just yet.

A lot has been said in this thread, @rashil2000's original issue will be tackled in #275 as I'm working on that right now. Redundant ANSI escape sequences have been taken out by @lnu, which hopefully makes things a bit more efficient. I'll create a POC to see of the syscalls on Windows can be bypassed, but no guarantees there until proven otherwise.

JanDeDobbeleer added a commit that referenced this issue Dec 27, 2020
@JanDeDobbeleer
Copy link
Owner

@rashil2000 once v3.65.0 is done building, can you validate this resolves your issue?

@rashil2000
Copy link
Author

Unfortunately, I can still hear the terminal bell when oh-my-posh (v3.65.0) is injected in Clink. Same as before.

@JanDeDobbeleer
Copy link
Owner

Wait, the bell? That wasn't mentioned before, right? This fix was for the clear EOL.

@JanDeDobbeleer
Copy link
Owner

JanDeDobbeleer commented Dec 27, 2020

Ok, so, I just reread the entire thread in 39 and see the mention of the bell and indeed that's because of setting the title. Setting console_title to false will resolve that.

@JanDeDobbeleer
Copy link
Owner

@rashil2000 can you confirm that the original issue as described 40 is now gone?

@chrisant996
Copy link
Contributor

chrisant996 commented Dec 27, 2020

oh-my-posh takes around 2 to 3 seconds to run on Windows

@chrisant996 that's definitely not normal. It runs below a second on my machines (and also confirmed by others). The only time this occurred, it disappeared after a reboot of the user, and as far as I know hasn't returned just yet.

Stopwatch says it's around 1 second. I guess it feels like 3 seconds because Lua scripts in Clink can do the same work (including git enhancements) in around 1/4 second, but that's to be expected since they don't have to load a large binary or initialize internal runtime frameworks, etc.

Redundant ANSI escape sequences have been taken out by @lnu, which hopefully makes things a bit more efficient.

Cool, sounds great.

It sounds like all of the issues are well understood, so at this point I think there's no need for me to collect/upload additional information.

@JanDeDobbeleer
Copy link
Owner

@chrisant996 thanks for taking the time to assist on this one. It clarified a lot and made us improve things 🙏🏻

@rashil2000
Copy link
Author

Setting console_title to false will resolve that.

How do I set this?

@lnu
Copy link
Contributor

lnu commented Dec 28, 2020

"console_title": false ìn the general settings of the json config file.

@rashil2000
Copy link
Author

@rashil2000 can you confirm that the original issue as described 40 is now gone?

Yes, no error bell now!

Thanks!

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

No branches or pull requests

4 participants