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

Trim leading and trailing whitespace, by default; closes #211 #326

Merged
merged 2 commits into from Apr 8, 2017

Conversation

Projects
None yet
2 participants
@jennybc
Member

jennybc commented Apr 7, 2017

This is the same PR you reviewed and approved already (#325), plus one commit. Why a new PR? Minor Git snafu.

First commit is the same.

Second commit deals with this: if trim_ws = TRUE then whitespace should be trimmed prior to any comparison with the na strings.

@jennybc jennybc requested a review from hadley Apr 7, 2017

@jennybc

This comment has been minimized.

Member

jennybc commented Apr 7, 2017

Also, you're going to see more test sheets (re-)named like so: tests/testthat/sheets/whitespace-xlsx.xlsx. It looks silly, but when you have the workbook open in Excel it is damn near impossible to tell if you're working on the xls or xlsx version. Repeating the extension in the filename makes it obvious.

@jennybc

This comment has been minimized.

Member

jennybc commented Apr 7, 2017

BTW this matches readr's behaviour re: na and trim_ws: trim, then check for NAs.

@hadley

hadley approved these changes Apr 8, 2017

@jennybc jennybc merged commit 00a8891 into tidyverse:master Apr 8, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jennybc jennybc deleted the jennybc:trim-whitespace branch Apr 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment