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

refactor: Move ANSI escaping into its own functions #866

Merged
merged 9 commits into from Feb 6, 2020
Merged

refactor: Move ANSI escaping into its own functions #866

merged 9 commits into from Feb 6, 2020

Conversation

chipbuster
Copy link
Contributor

Description

Moves ANSI colorseq escape functionality into its own functions and adds tests.

Motivation and Context

One of the proposed changes in #865 involved escaping an ANSI code from within print.rs. Currently, escaping these sequences is done inside of a closure inside module, which prevents it from being used elsewhere.

This change allows us to escape unprintable characters from anywhere inside starship, as well as allowing a greater variety of escape sequences and tests specific to the escape parser (as opposed to just the module string generator).

Types of changes

Refactoring

Screenshots (if appropriate):

How Has This Been Tested?

  • I have tested using MacOS
  • I have tested using Linux
  • I have tested using Windows

Checklist:

  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@chipbuster chipbuster changed the title refactor: Move escaping into its own functions refactor: Move ANSI escaping into its own functions Jan 22, 2020
Copy link
Contributor

@jonstodle jonstodle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matchai
Copy link
Member

matchai commented Feb 1, 2020

LGTM if we can get the conflicts sorted out 👍

@chipbuster
Copy link
Contributor Author

Oh....yeahhhhh 😅

So I tried to rebase this after the shell enum change, failed miserably, and somehow wound up with a pile of code that had merge conflict markers committed into it. I had planned to try again at some point, and I guess I just forgot.

@chipbuster
Copy link
Contributor Author

headdesk

What the heck did I just do??

@matchai matchai merged commit e3a30c3 into starship:master Feb 6, 2020
dagbrown pushed a commit to dagbrown/starship that referenced this pull request Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants