-
Notifications
You must be signed in to change notification settings - Fork 367
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
fix(stdin): trim space instead of \n #761
Conversation
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>
@@ -30,7 +30,7 @@ func Read() (string, error) { | |||
} | |||
} | |||
|
|||
return strings.TrimSuffix(b.String(), "\n"), nil | |||
return strings.TrimSpace(b.String()), nil |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
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 🤔
@@ -30,7 +30,7 @@ func Read() (string, error) { | |||
} | |||
} | |||
|
|||
return strings.TrimSuffix(b.String(), "\n"), nil | |||
return strings.TrimSpace(b.String()), nil |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
we were trimming an ending \n, but it would then break a \r\n sequence, causing misrenders.
this fixes it
closes #682