Handling NULL inside braces

[echasnovski]

For me this is unexpected behavior. I might understand this output if one of glued strings is NULL (as after #46) but this is a case of evaluating R code.

library(glue)

world <- NULL

res <- glue("Hello {world}!")
# For me the expected result is "Hello !"
str(res)
#> Classes 'glue', 'character'  chr(0)

Created on 2018-07-23 by the reprex package (v0.2.0).

Sorry if this is considered a duplicate of #45 or #84.

[echasnovski]

I see that this might be handled with transformers but may be this should be a default?

library(glue)

null_transformer <- function(text, envir) {
  out <- identity_transformer(text, envir)
  if (is.null(out)) {
    return("NULL")
  }
  
  out
}

world <- NULL

glue("Hello {world}!", .transformer = null_transformer)
#> Hello NULL!

Created on 2018-07-24 by the reprex package (v0.2.0).

[cfhammill]

Two more ways you can hit this bug:

str(glue("hi {}"))

 #> 'glue' chr(0) 

expected return

#> 'glue' chr "hi "

I encountered this with an empty else in a conditional

str(glue("hi {if(FALSE) 'there'}")

#> 'glue' chr(0)

This example also breaks reprex somehow, but that is question for another day.

[cfhammill]

@echasnovski, I think the NULL transformer should do as we both expected and return an empty string instead of text "NULL".

[echasnovski]

Yeah, maybe. However, using "NULL" proved to be a better choice in certain use cases. For example, when printing the current value of some variable. Useful in error messages.

My feeling is that empty string might be a better default choice, and null_transformer() pattern should be documented as alternative somewhere.

[cfhammill]

Great point, yes NULL is sometimes desirable. Also agree empty string default + well documented null_transformer is a good approach

[cfhammill]

do you want to promote this to a PR? If not I'm happy to. @jimhester what do you think?

[echasnovski]

I believe, that this behavior is a design choice (see #28, #45, and b945cd1).
Nevertheless, I don't have any intentions to make a PR here, so feel free to do it.

[cfhammill]

you're totally right that the NULL annihilating your results does seem to be by design. And yeah, seems like a pretty trivial addition to the transformers vignette

[jimhester]

You are right this is a design choice, but I also agree that a null transformer should be added to the vignette.

If I might suggest an addition, have the null transformer take a string for the text to insert.

null_transformer <- function(str = "NULL") {
  function(text, envir) {
    out <- glue::identity_transformer(text, envir)
    if (is.null(out)) {
      return(str)
    }
  
    out
  }
}
glue::glue("hi {}", .transformer = null_transformer("{NULL}"))
#> hi {NULL}

^{Created on 2018-10-05 by the reprex package (v0.2.1)}

[grayskripko]

this issue is one year old.
I have a strong feeling that this is a bug actually. If you don't want to return a part of a string without NULL you can throw an error at least.

[LukasWallrich]

I would also find it very helpful if glue issued a warning when NULLs are present. However, after getting frustrated with tracking down the NULLs in my complex code, I figured out that a transformer can create the warnings I would like.

If you do not want to include such warnings into glue, maybe something like this would be helpful in the vignette?

warn_null <- function(text, envir, val = "NULL", ...) {
  res <- glue::identity_transformer(text, envir)
  if (is.null(res)) {
    warning("glue item evaluates to NULL: ", text, call. = FALSE, immediate. = TRUE)
    val
  } else {
    res
  }
}