search

gesistsa / adaR

:computer: wrapper for ada-url a WHATWG-compliant and fast URL parser written in modern C++
https://gesistsa.github.io/adaR/
Other
26 stars 2 forks source link

Fix #26 #28

Closed chainsawriot closed 1 year ago

schochastics commented 1 year ago

thanks!

chainsawriot commented 1 year ago

@schochastics More tests will be added later for the PR #3.

Suggestion:

  1. Do you want to consolidate the documentation of all ada_has_* in one documentation.
  2. I found that utf8::as_utf8 is probably not needed on modern enough R. It can cut the running time by ~10%. But you would at least need to set the R requirement to 4.2 (for those using Windows).
schochastics commented 1 year ago

@chainsawriot

  1. yes. If you fix the NULL thing, do you mind doing that in the same PR?
  2. Will think about that but I think it is a good idea. I also would like to get rid of the vapply for the length which is a runtime killer
chainsawriot commented 1 year ago

@schochastics Good, will do 1.

  1. The length thing is partially fixed by calculating the char* length in C++.
schochastics commented 1 year ago

@chainsawriot yes I realized that too just now.