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

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

bmeyers22
Copy link

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

// brute force for the moment
resultStr = strings.TrimSuffix(line, "\n")
resultStr = strings.TrimSuffix(resultStr, "\n")
Copy link
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

Could this be merged cc @tcnksm ?

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

CAFxX commented May 1, 2018

@tcnksm

image

@perdasilva
Copy link

Will this ever be merged?

@zishang520
Copy link

...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