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

glue_col to work without attaching crayon ? (+ a few comments) #138

Closed
moodymudskipper opened this issue Jun 26, 2019 · 4 comments · Fixed by #241
Closed

glue_col to work without attaching crayon ? (+ a few comments) #138

moodymudskipper opened this issue Jun 26, 2019 · 4 comments · Fixed by #241

Comments

@moodymudskipper
Copy link
Contributor

moodymudskipper commented Jun 26, 2019

The doc seems to imply that only crayon functions can be used with glue_col, considering this I have a few comments:

  1. If it can in some circumstances make sense to use a function that is not from crayon, could it be documented ?

  2. As crayon functions will be the ones mostly used in any case, isn't it redundant to require the pakage to be attached, couldn't glue_col access crayon's namespace ?

  3. glue::glue_col("{crayon::blue hello}") triggers the following error, which means there's no simple way out if crayon isn't attached, isn't it wrong to "force" the user to attach a package if it can be avoided ? (note : glue_col("{yellow$bgMagenta$bold Hello} world!") doesn't work either, maybe it could be supported by the same token ?)

Error in parse(text = code, keep.source = FALSE) :
:1:14: unexpected symbol
1: crayon::blue hello

4 glue::glue_col("{blue hello}") triggers the following error when crayon is not attached. the doc is very clear but the error itself might be clearer if it mentions the need to attach crayon. If you consider my point 2. this point becomes irrelevant, and a helpful error would suggest installing crayon if it's not installed.

Error in get(fun, envir = envir, mode = "function") :
object 'blue' of mode 'function' was not found

My use case is that I want to wrap glue_col in a package, and I'd rather not attach crayon. My work around is to call glue_col wrapped inside your own withr::with_package("crayon", *code using glue_col*) but it means one more dependency and is not obvious.

@jimhester
Copy link
Collaborator

jimhester commented Jun 26, 2019

In a package import the specific functions from crayon you want with #' @importFrom crayon blue.

For custom functions, assign them to an object and then use that, one of the examples shows this usage white_on_grey <- bgBlack $ white

Here are your examples

my_color <- crayon::yellow$bgMagenta$bold
glue::glue_col("{my_color Hello} world!")

blue <- crayon::blue
glue::glue_col("{blue hello}")

Screen Shot 2019-06-26 at 9 07 01 AM

@moodymudskipper
Copy link
Contributor Author

Thanks it makes sense, my workaround was quite convoluted :).

@moodymudskipper
Copy link
Contributor Author

In hindsight Jim this is not good enough, check out this more realistic example :

  • I have a function in my package that will wrap glue_col, to print text in a box.
  • I don't want to attach crayon nor import it by default as I have a lot of functions depending on a lot of packages and wish to limit dependencies
  • thus I'd rather add crayon to Suggests and check that the package is installed and then test with requireNamespace if the package is available whenever my function is called.

Here's what I would like to work :

box_print <- function(txt){
  if(!requireNamespace("glue")) 
    stop("The package glue must be installed to use box_print")
  if(!requireNamespace("crayon")) 
    stop("The package crayon must be installed to use box_print")

  txt <- glue::glue_col(txt)
  line <- strrep("#", crayon::col_nchar(txt)+4)
  cat(sep="", "\n", line, "\n# ", txt, " #\n", line ,"\n")
}

box_print("foo")
#> Loading required namespace: glue
#> Loading required namespace: crayon
#> 
#> #######
#> # foo #
#> #######
box_print("{green hello} {blue world}")
#> Error in get(fun, envir = envir, mode = "function"): object 'green' of mode 'function' was not found

# oops, I need to attach
library(crayon)
box_print("{green hello} {blue world}")
#> 
#> ###############
#> # hello world #
#> ###############

If I don't want to attach or import I need a workaround, which is a bit more complex than it could be given r-lib/withr#107 :).

detach("package:crayon")
box_print <- function(txt){
  if(!requireNamespace("glue")) 
    stop("The package glue must be installed to use box_print")
  if(!requireNamespace("crayon")) 
    stop("The package crayon must be installed to use box_print")
  if("crayon" %in% search())
  {
    txt <- glue::glue_col(txt)
  } else {
    withr::with_package("crayon", txt <- glue::glue_col(txt))
  }
    
  line <- strrep("#", crayon::col_nchar(txt)+4)
  cat(sep="", "\n", line, "\n# ", txt, " #\n", line ,"\n")
}

box_print("{green hello} {blue world}")
#> 
#> ###############
#> # hello world #
#> ###############
"crayon" %in% search()
#> [1] FALSE

Created on 2019-06-26 by the reprex package (v0.3.0)


The function glue_col has the following code :

function (..., .envir = parent.frame(), .na = "NA") 
{
    loadNamespace("crayon")
    glue(..., .envir = .envir, .na = .na, .transformer = color_transformer)
}

It seems to me loadNamespace is not doing anything here since "crayon" needs to be attached anyway.

@jimhester
Copy link
Collaborator

Just import the functions you need, crayon is an R only package with no non-base dependencies, you should not be worried about including it.

If you really want to do what you propose just assign the functions you want to use in whatever environment you want to use them in.

jennybc added a commit that referenced this issue Nov 24, 2021
jennybc added a commit that referenced this issue Nov 24, 2021
jennybc added a commit that referenced this issue Nov 24, 2021
* Give (potential) crayon calls special treatment

Fixes #196, fixes #138 (for the links)

* Add NEWS bullet

* Refactor tests; cover the crayon-not-attached scenario
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 a pull request may close this issue.

2 participants