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

Don't trim whitespace in input text needlessly #63

Merged
merged 2 commits into from
Sep 17, 2021

Conversation

Caesarovich
Copy link
Collaborator

@Caesarovich Caesarovich commented Sep 13, 2021

This PR disables text trimming where it is unneeded. It comes in response to #62

At line 233 there was this line of code:

let contentWidth = widestLine(wrapAnsi(text, columns - BORDERS_WIDTH, {hard: true})) + padding.left + padding.right;

This line has for goal to determine the columns that the text would take with the padding.
The problem is that by default the wrapAnsi function trims the lines, hence removing the whitespaces in #62 . This is certainly an unwanted behavior because it alters the user input with no reason.
So I added the trim: false option in the wrapAnsi function.

I also needed to adapt my tests because it is not something I realized when making the code refactoring PR.


Fixes #62

@Caesarovich
Copy link
Collaborator Author

// @sindresorhus

@sindresorhus
Copy link
Owner

I also needed to adapt my tests because it is not something I realized when making the code refactoring PR.

I think it would make sense to switch some of the tests to snapshot tests. It would be easier to update and easier to see the changes.

@sindresorhus sindresorhus changed the title Text trim fix Don't trim whitespace in input text needlessly Sep 17, 2021
@sindresorhus sindresorhus merged commit 760e247 into sindresorhus:main Sep 17, 2021
@Caesarovich
Copy link
Collaborator Author

Caesarovich commented Sep 17, 2021

I also needed to adapt my tests because it is not something I realized when making the code refactoring PR.

I think it would make sense to switch some of the tests to snapshot tests. It would be easier to update and easier to see the changes.

What do you mean by that ? Tests that compare results between two versions of the code ?

@sindresorhus
Copy link
Owner

@Caesarovich
Copy link
Collaborator Author

https://github.com/avajs/ava/blob/main/docs/04-snapshot-testing.md

Oh yeah this is even better, it's really sweet. Do you want me to work on that ?

@sindresorhus
Copy link
Owner

Sure, if you want.

@Caesarovich
Copy link
Collaborator Author

Ok, working on that for tomorrow ;)

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.

cropping leading whitespace in new version ??
2 participants