Shougo / dein.vim

:zap: Dark powered Vim/Neovim plugin manager
MIT License
3.42k stars 197 forks source link

Change ANSI escape to \033 and get rid of -e flag #480

Closed samyilin closed 1 year ago

samyilin commented 1 year ago

Problems summary

  1. \x1B seem to not function properly as an escape sequence on WSL.
  2. since escape sequence is in printf, echo -e does not do anything useful. Example attached below. Please check on other platforms.
  3. In addition, flags in echo seem to be outputted for some reason.

I do realize that I could submit a pull request to resolve it, but it seems that recent changes were made by @santosned , so I wanted to make sure we are okay with any changes.

Expected

Escape character to be recognized and codebase simplified. I do recognize this is more of a Windows problem, but from a portability perspective, it is better to have portable solutions. "Portable" solutions such as pfetch seems to use \033, and this program (pfetch) works flawlessly on WSL.

Environment Information (Required!)

Ubuntu in WSL over Windows Terminal.

Screen shot (if possible)

This screenshot showcases what would happen if I ran ./install with \x1B as escape sequence in Windows Terminal (Ubuntu in WSL): image The following screenshots showcase if I:

  1. replace \x1B with \033
  2. get rid of flags in echo (no echo -e, no echo -ne)
  3. replace 30 color with 32 just so we can see some result:

image image If I do 1 and 3 but not 2, then it would output image image

This screenshot shows that when passing result of printf to echo, -e flag is unnessary. Please double check on other platforms. image

Upload the log messages by :redir and :message (if errored)

ghost commented 1 year ago

I didn't expect this problem with echo since Octal, Unicode, or Hexadecimal ANSI formats should be supported by both echo or printf on bash or sh.

As showed running on Bourne Shell:

Echo command ![image](https://user-images.githubusercontent.com/99996904/205457940-74dfd36e-ce44-4f2c-a38a-4d12075d4191.png)
Printf command ![image](https://user-images.githubusercontent.com/99996904/205458257-08b36136-e85b-403e-ab2b-b138a0feb953.png)

Conclusion

As you said this must be related with an specific version of the shell you're using.

Nevertheless, I see your point. I'll close my initial PR and work on an improved version of the formated messages. Thanks for the contribution @samyilin.

samyilin commented 1 year ago

Thanks @santosned! I don't believe this is related to Bash either, I tried it on both Bash and busybox sh and they produced the same illegible result. So this could related to how it's been handelled by Windows or Windows Terminal or your nested usage of echo and printf? More testing could be needed.

This problem is only present if I use the installation script. If I ran your example code in Bash (your comment here), I get the same result as yours. So not sure what the problem is there either.

I will close this issue ( or someone else) if your new pull request fixes this issue. Thanks!

ghost commented 1 year ago

So this could related to how it's been handelled by Windows or Windows Terminal or your nested usage of echo and printf? More testing could be needed.

I guess it's both options, the usage of nested echo and printf on Windows.

I've other script that I didn't used this nested method and are being used in production currently without any problems like this.

This case I tested on Linux only, so I couldn't known how it would go on Windows.

This sure is a learning experience.

Thanks again @samyilin

And you don't need to close the issue by hand.

If you tested and it's working leave a comment or emoji reaction in the pull request, so we known it's fixed.

The issue is automatically closed when an PR related to it is merged.

samyilin commented 1 year ago

Thanks for the knowledge @santosned, I haven't raised issues before, so pardon my closing issue by hand. I tested your code and it works great! I've learned a lot about terminal and ANSI codes in this process. Thank you again for the effort!