Skip to content

fix(stdin): trim space instead of \n#761

Merged
caarlos0 merged 1 commit intomainfrom
in
Dec 11, 2024
Merged

fix(stdin): trim space instead of \n#761
caarlos0 merged 1 commit intomainfrom
in

Conversation

@caarlos0
Copy link
Copy Markdown
Contributor

we were trimming an ending \n, but it would then break a \r\n sequence, causing misrenders.

this fixes it

closes #682

we were trimming an ending \n, but it would then break a \r\n sequence, causing misrenders.

this fixes it

closes #682

Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>
Comment thread internal/stdin/stdin.go
}

return strings.TrimSuffix(b.String(), "\n"), nil
return strings.TrimSpace(b.String()), nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So you are also trimming on the left.

Please confirm if it intended and OK

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point. If it matters at all, assuming we'd want strings.TrimRight() to match strings.TrimSuffix.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thought about that, I'm not sure why we were trimming the eof \n... in any case, it's probably fine 🤔

Comment thread internal/stdin/stdin.go
}

return strings.TrimSuffix(b.String(), "\n"), nil
return strings.TrimSpace(b.String()), nil
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would this trim \r\n suffixes?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes!

@caarlos0 caarlos0 merged commit b0c9c58 into main Dec 11, 2024
@caarlos0 caarlos0 deleted the in branch December 11, 2024 00:27
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.

Unexpected handling of ascii input text

4 participants