Skip to content

fix: drop strip-ansi in favor of native stripVTControlCharacters #4095

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
Merged
JamesHenry merged 3 commits into from
Nov 4, 2024
Merged

fix: drop strip-ansi in favor of native stripVTControlCharacters #4095

JamesHenry merged 3 commits into from
Nov 4, 2024

Conversation

@mcmxcdev
Copy link
Contributor

@mcmxcdev mcmxcdev commented Oct 16, 2024

Description

  • uninstall strip-ansi dependency from multiple package.json
  • replace strip-ansi import with util.stripVTControlCharacters in wide-truncate.ts
  • replace strip-ansi import with util.stripVTControlCharacters in index.spec.ts

Motivation and Context

References e18e/ecosystem-issues#122

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (change that has absolutely no effect on users)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@mcmxcdev
chore: drop strip-ansi in favor of native stripVTControlCharacters
e40b58e 
- uninstall strip-ansi dependency from multiple package.json
- replace strip-ansi import with util.stripVTControlCharacters in wide-truncate.ts
- replace strip-ansi import with util.stripVTControlCharacters in index.spec.ts
@mcmxcdev mcmxcdev marked this pull request as ready for review October 16, 2024 20:29
@nx-cloud
Copy link

nx-cloud bot commented Oct 31, 2024 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 1d45c76. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 8 targets

Sent with 💌 from NxCloud.

@JamesHenry
Copy link
Member

JamesHenry commented Oct 31, 2024

Thank you @mcmxcdev! Looks like the snapshot change you made causes the tests to fail. Was that unintentional?

JamesHenry
JamesHenry requested changes Oct 31, 2024
View reviewed changes
Copy link
Member

@JamesHenry JamesHenry left a comment

Choose a reason for hiding this comment

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

See comment

@JamesHenry JamesHenry changed the title chore: drop strip-ansi in favor of native stripVTControlCharacters fix: drop strip-ansi in favor of native stripVTControlCharacters Oct 31, 2024
@JamesHenry
Copy link
Member

JamesHenry commented Oct 31, 2024

I have marked this as a fix because it affects the runtime, and even though it should be like for like I think it's good to call it out in the release notes (which will happen automatically for a fix)

@mcmxcdev
Copy link
Contributor Author

mcmxcdev commented Oct 31, 2024 •
edited
Loading

Thank you @mcmxcdev! Looks like the snapshot change you made causes the tests to fail. Was that unintentional?

This was definitely unintenional and was caused by this setting in VSCode:

image

Not gonna solve it for every dev, but it might be a good idea to introduce a settings.json in the repo that disables this setting.

I readded the trimmed whitespace in 1d45c76 (#4095)

@JamesHenry JamesHenry merged commit 9e4ac9c into lerna:main Nov 4, 2024
@JamesHenry
Copy link
Member

JamesHenry commented Nov 4, 2024

Thank you @mcmxcdev!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JamesHenry JamesHenry JamesHenry requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@mcmxcdev @JamesHenry