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

Handle trailing newline in REDCap checkbox options #502

Closed
wants to merge 1 commit into from

Conversation

BlairCooper
Copy link
Contributor

fixes #501

  • Modified pattern_checkboxes regular expression to allow for case where there is no space in front of the pipe character in the Lookbehind and Lookahead expressions, and allow the space(s) after the comma to be optional.

  • Created test case to verify correct handling when a checkbox options has a trailing newline.

  • Created test case to verify correct handling when the space is missing after the comma for an option.

fixes OuhscBbmc#501

- Modified pattern_checkboxes regular expression to allow for case where
  there is no space in front of the pipe character in the Lookbehind and
  Lookahead expressions, and allow the space(s) after the comma to be
  optional.

- Created test case to verify correct handling when a checkbox options
  has a trailing newline.

- Created test case to verify correct handling when the space is missing
  after the comma for an option.
@wibeasley
Copy link
Member

@BlairCooper, I've tagged you about this in two previous issues (#501 & #500). Can you respond to those?

@wibeasley
Copy link
Member

@BlairCooper, sorry I missed your previous response in the other thread.

For this PR, the regex isn't working if the id is a negative number. I don't want to discourage you here if you're passionate about continuing a home-grown regex. But I think the two-step approach (of strsplit() and readr::read_csv()) will be more robust to most of these corner cases.

Screenshot of REDCap designer:
image

String for test cases:

"1, American Indian/Alaska Native | -2, Asian | 3, Native Hawaiian or Other Pacific Islander | 4, Black or African American | 5, White | 66, Unknown / Not Reported" |>

@wibeasley
Copy link
Member

@BlairCooper, it sounds like you're ok with the approach of #504, so I'm closing this PR. Please tell me if I'm wrong. I've moved this PR's new scenarios to the list of tests.

@wibeasley wibeasley closed this Jul 15, 2023
@BlairCooper BlairCooper deleted the checkbox_choices branch July 15, 2023 23:40
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.

Trailing newline in Checkbox option causes choice to omitted
2 participants