Skip to content

Fixes trimming for windows. \r\n are not trimmed when using TrimSuffix#14

Open
bmeyers22 wants to merge 5 commits intotcnksm:masterfrom
Workiva:master
Open

Fixes trimming for windows. \r\n are not trimmed when using TrimSuffix#14
bmeyers22 wants to merge 5 commits intotcnksm:masterfrom
Workiva:master

Conversation

@bmeyers22
Copy link
Copy Markdown

In regards to #12 this is the fix. The issues were:

  • TrimSuffix for whatever reason does not trim \r\n, but Trim does
  • The brute force line wasn't taking the previous lines output, but just stripping the original value of \n from the \r\n

Comment thread read.go
resultStr = strings.Trim(line, LineSep)
// brute force for the moment
resultStr = strings.TrimSuffix(line, "\n")
resultStr = strings.TrimSuffix(resultStr, "\n")
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Note: I'm not sure of the reasoning for this line so I left it in, but I'm guessing you needed it to get rid of new lines no matter what since they probably weren't working for windows. Using Trim instead of TrimSuffix should solve this problem, so you may be able to remove this line

@valarauca
Copy link
Copy Markdown

Could this be merged cc @tcnksm ?

@CAFxX CAFxX mentioned this pull request May 1, 2018
@CAFxX
Copy link
Copy Markdown

CAFxX commented May 1, 2018

@tcnksm

image

@perdasilva
Copy link
Copy Markdown

Will this ever be merged?

@zishang520
Copy link
Copy Markdown

...9102 years

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.

7 participants