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

Give (potential) crayon calls special treatment #241

Merged
merged 3 commits into from
Nov 24, 2021
Merged

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Nov 24, 2021

Fixes #196, fixes #138, fixes #221 (both already closed, but mentioning to get the links)

Previously glue_col() was in some intermediate state between giving special treatment to crayon's coloring functions and not doing so. The docs (and source) strongly suggested glue_col("{blue whatever}") would "just work" and yet that call definitely did not work.

In this PR, we admit that crayon is special and simple usage of its colouring functions will now work without, e.g., attaching crayon or, in a package, importing its functions. Fancier compound styling functions, such as the concluding example of glue_col() still require attaching or importing or the like, but I think that's (sort of) obvious. And much more rare usage, in any case.

@jimhester
Copy link
Collaborator

LGTM, do you think we should add any tests for this?

@jennybc
Copy link
Member Author

jennybc commented Nov 24, 2021

I was thinking about tests. Given that we generally test in the presence of suggested packages, I guess a test of the "crayon absent" scenario will effectively be a manual test, but it won't hurt to record it officially and have an appropriate skip.

@jimhester
Copy link
Collaborator

You could mock requireNamespace("crayon") as FALSE to test that code path if you wanted I guess.

@jennybc
Copy link
Member Author

jennybc commented Nov 24, 2021

The whole diff is covered now. I refactored the tests to use more standard testthat syntax.

@jennybc jennybc merged commit 72b4f6b into main Nov 24, 2021
@jennybc jennybc deleted the glue-col-usability branch November 24, 2021 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants