urltools icon indicating copy to clipboard operation
urltools copied to clipboard

fix to the issue #90

Open ekamioka opened this issue 6 years ago • 6 comments

Hey @Ironholds

AlexCP said maybe I can help, so here is my PR in hope to solve it.

I added new tests to ensure I am doing the thing (and also not breaking the codebase). Running results was:

==> Testing R file using 'testthat'

✔ | OK F W S | Context
✔ | 55       | URL parsing tests [0.2 s]

══ Results ════════════════════
Duration: 0.3 s

OK:       55
Failed:   0
Warnings: 0
Skipped:  0

Test complete

ekamioka avatar Aug 29 '18 14:08 ekamioka

Does this work nicely on Windows machines as well? I know there are historical legacy issues with getting R to play nice with the regex header there.

Ironholds avatar Aug 30 '18 17:08 Ironholds

@Ironholds I know nothing about RCpp packages, but can we add the --std=c++11 flag to the build process so it can add that independently if I do not have it on my RMakevars ?

alexcpsec avatar Dec 13 '18 16:12 alexcpsec

Hm; I don't remember either. I think the DESCRIPTION should now let you specify? It's been a while.

Ironholds avatar Dec 14 '18 00:12 Ironholds

@Ironholds I know nothing about RCpp packages, but can we add the --std=c++11 flag to the build process so it can add that independently if I do not have it on my RMakevars ?

I've just tested and adding PKG_CXXFLAGS=-std=c++11 to .R/Makevars or setting Sys.setenv("PKG_CXXFLAGS"="-std=c++11") will do the work.

PS. gcc version must be greater or equals to 4.9

ghost avatar Dec 17 '18 16:12 ghost

And does R now require C++11 support? (that is, is it likely to be on target machines?)

Ironholds avatar Dec 18 '18 16:12 Ironholds

Nah, the thing here is that Eduardo's proposed fix was to use <regex> which is a C++11 construct. So instead of forcing the user to manually add C++11 support to all his packages (say in RMakevars), I was wondering if we could just do it for the package itself.

Another option is to solve this issue without <regex> :)

On Tue, Dec 18, 2018 at 8:17 AM Os Keyes [email protected] wrote:

And does R now require C++11 support? (that is, is it likely to be on target machines?)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_Ironholds_urltools_pull_91-23issuecomment-2D448277587&d=DwMCaQ&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=0RF4UdPYmeY-L_O1i6y7mZxXvdEOLIArMs1cG7-uYlw&m=VVRFFMXJ0FwxxQ_02jnGwYtsM6cIzdjptI_8t19I8bU&s=d6dcF0-lMjbXCDYcJrTzFrlFPxsHa1TzgiP4x5bXteA&e=, or mute the thread https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_notifications_unsubscribe-2Dauth_ADDeUyRLdaddIwBQmEdmQpLH6pVufSLGks5u6RUKgaJpZM4WRrg5&d=DwMCaQ&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=0RF4UdPYmeY-L_O1i6y7mZxXvdEOLIArMs1cG7-uYlw&m=VVRFFMXJ0FwxxQ_02jnGwYtsM6cIzdjptI_8t19I8bU&s=mO2a6ZxxxyvyslNH3MqZ4U21geQQdnfl0K-FKeCVq9o&e= .

--

Alexandre Pinto

Product Manager and Distinguished Engineer

Security Services Global Products and Solutions

M +1 (650) 441-2081

[email protected] | [email protected]

*Advance Notice of Parental Leave (if Baby follows agreed upon schedule): *

Jan 11, 2019 - Feb 4, 2019

Feb 18, 2019 - March 25, 2019

alexcpsec avatar Dec 18 '18 17:12 alexcpsec