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

url_decode2 is not NA aware #47

Closed
chainsawriot opened this issue Sep 27, 2023 · 5 comments
Closed

url_decode2 is not NA aware #47

chainsawriot opened this issue Sep 27, 2023 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@chainsawriot
Copy link
Collaborator

adaR::url_decode2(NA)
#> [1] "NA"

Created on 2023-09-27 with reprex v2.0.2

Fixing this can also enable decoding in C++.

@chainsawriot chainsawriot added the bug Something isn't working label Sep 27, 2023
@chainsawriot chainsawriot self-assigned this Sep 27, 2023
@schochastics
Copy link
Member

schochastics commented Sep 27, 2023

@chainsawriot Do you mean something like

CharacterVector Rcpp_ada_get(const CharacterVector& url_vec, std::function<ada_string(ada_url)> func, bool decode) {
  unsigned int n = url_vec.length();
  CharacterVector out(n);
  for (int i = 0; i < url_vec.length(); i++) {
    String s = url_vec[i];
    const char* input = s.get_cstring();
    ada_url url = ada_parse(input, std::strlen(input));
    if (!ada_is_valid(url)) {
      out[i] = NA_STRING;
    } else {
      out[i] = charsub(func(url));
    }
  }
  if(decode){
    return(url_decode2(out))
  }else{
    return (out);
}
}

@chainsawriot
Copy link
Collaborator Author

yeah. I am eyeing on removing all of these

https://github.com/schochastics/adaR/blob/14692c751ac2fdbc97caa5b491357788d51eb7f6/R/parse.R#L20-L42

@chainsawriot
Copy link
Collaborator Author

url_decode2(NULL)

kills R.

@chainsawriot
Copy link
Collaborator Author

chainsawriot commented Sep 27, 2023

@schochastics Would you mind me changing url_decode2 with an R wrapper to filter the NULL case?

CharacterVector url_decode2(CharacterVector& url) {
  if (url.isNULL()) {
    CharacterVector output(0);
    return output;
  }
}

url_decode2(NULL) still kills this. Tried also Rf_isNull. It can't even pass the first line.

@schochastics
Copy link
Member

@chainsawriot no go ahead and make that change

chainsawriot added a commit to chainsawriot/adaR that referenced this issue Sep 27, 2023
Still not working
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants