Skip to content

Fix for dev stringr#157

Merged
flying-sheep merged 2 commits into
IRkernel:masterfrom
hadley:dev-stringr
Nov 1, 2022
Merged

Fix for dev stringr#157
flying-sheep merged 2 commits into
IRkernel:masterfrom
hadley:dev-stringr

Conversation

@hadley
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

hadley commented Oct 31, 2022

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

@flying-sheep
Copy link
Copy Markdown
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.

2 participants