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

permit single quotes if they quote literal double quotes #28

Merged
merged 3 commits into from
Jan 5, 2015

Conversation

jackwasey
Copy link

A legitimate use of a single quote pair in R is to quote a literal double quote. This tiny patch allows this. It's not immediately clear to me how to test a negative with your check_lint tests. Thanks.

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.01%) when pulling 1b8c98b on jackwasey:master into 1f85019 on jimhester:master.

@jimhester
Copy link
Member

Thank you very much for the patch.

Could you make the following changes.

  1. Please use rex to construct the regular expression.

    • The equivalent rex expression to the regex you wrote is
    rex(start, single_quote, any_non_double_quotes, single_quote, end)
  2. Please add at least one additional test to tests/testthat/test-single_quotes_linter.R showing the new functionality.

    • You can test that a given linter does not return any lints by passing NULL as the second argument.
    • So an acceptable test would be
    expect_lint('quote "something"', NULL, single_quotes_linter)

Thanks!

@coveralls
Copy link

Coverage Status

Coverage decreased (-5.01%) when pulling b1a9d4b on jackwasey:master into 1f85019 on jimhester:master.

@jackwasey
Copy link
Author

Think that should do it. Thanks.

jimhester added a commit that referenced this pull request Jan 5, 2015
permit single quotes if they quote literal double quotes
@jimhester jimhester merged commit c19216f into r-lib:master Jan 5, 2015
@jimhester
Copy link
Member

Thanks!

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.

3 participants