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

Fix for dev stringr #157

Merged
merged 2 commits into from
Nov 1, 2022
Merged

Fix for dev stringr #157

merged 2 commits into from
Nov 1, 2022

Conversation

hadley
Copy link
Contributor

@hadley hadley commented Oct 20, 2022

Where str_view() now prefers a console display that uses cli for colouring

Where `str_view()` now prefers a console display that uses cli for colouring
@flying-sheep
Copy link
Member

flying-sheep commented Oct 21, 2022

Doesn’t work with the current version:

Error in `stringr::str_view("xy", "y", html = TRUE)`: unused argument (html = TRUE)

So either repr has to wait for the new stringr to get released (which would result in temporary breakage) or we make the code forward compatible without breaking compatibility with the current version.

You could just do a patch release 1.4.2 for stringr that accepts html = TRUE and throws an error on html = FALSE, then we can change the dependency version of repr to >= 1.4.2 and release a forward-compatible repr version before stringr 1.5 comes out.

@hadley
Copy link
Contributor Author

hadley commented Oct 21, 2022

Sorry, the stringr dev version is so old, I'd forgotten that this argument wasn't in the CRAN version.

Given that this is in a test, and I presume you're just using str_view() since it's a simple HTML widget, are you ok with just a version switch?

@flying-sheep
Copy link
Member

Ah right, didn’t notice this was just tests!

Sure, I’ll merge this once the new version is out, if that happens soon enough that I remember why I subscribed to tidyverse/stringr releases.

Thank you for helping out with the reverse deps!

* Conditionally use html = TRUE if needed
* Move skip_if_installed() checks
@hadley
Copy link
Contributor Author

hadley commented Oct 21, 2022

This patch should work regardless of the version of stringr, so you can merge/release whenever.

I haven't official kicked off the stringr release process, but I'll ping this PR when I do (unless it's obvious that an updated repr is already on CRAN).

@hadley
Copy link
Contributor Author

hadley commented Oct 31, 2022

FYI stringr is now scheduled for release to CRAN on Dec 2.

@flying-sheep
Copy link
Member

thank you for the info!

@flying-sheep flying-sheep merged commit 0b9abc9 into IRkernel:master Nov 1, 2022
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.

None yet

2 participants