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

Feature: Stricter addition #286

Closed
TimTaylor opened this issue Jan 11, 2023 · 5 comments · Fixed by #297
Closed

Feature: Stricter addition #286

TimTaylor opened this issue Jan 11, 2023 · 5 comments · Fixed by #297
Labels
feature a feature request or enhancement

Comments

@TimTaylor
Copy link

TimTaylor commented Jan 11, 2023

The binary addition operator currently has the following implementation

`+.glue` <- function(e1, e2) {
  glue(e1, e2, .envir = parent.frame())
}

Would it be desirable to be a little stricter here and maybe error if one of the inputs is not a character / glue object. It feels like the following would be better to error:

1L + glue::glue("y")
#> 1y

Created on 2023-01-11 with reprex v2.0.2

@hadley
Copy link
Member

hadley commented Jan 25, 2023

Reprex:

1L + glue::glue("y")
#> 1y

Created on 2023-01-25 with reprex v2.0.2

@hadley hadley added the feature a feature request or enhancement label Jan 25, 2023
@hadley
Copy link
Member

hadley commented Jan 26, 2023

And shouldn't it be vectorised?

"(" + glue::glue(c("y", "z")) + ")"
#> Error: All unnamed arguments must be length 1

Created on 2023-01-26 with reprex v2.0.2

@hadley
Copy link
Member

hadley commented Jan 27, 2023

And this seems wrong:

glue::glue("{x}", x = "{x}") + "y"
#> Error in eval(parse(text = text, keep.source = FALSE), envir): object 'x' not found

Created on 2023-01-26 with reprex v2.0.2

We're interpolating the string twice

hadley added a commit that referenced this issue Jan 27, 2023
@jennybc
Copy link
Member

jennybc commented Mar 17, 2023

Note to self, making the last example even more obvious

glue::glue("{x}", x = "{wut}")
#> {wut}
glue::glue("{x}", x = "{wut}") + "y"
#> Error in eval(parse(text = text, keep.source = FALSE), envir): object 'wut' not found

Created on 2023-03-17 with reprex v2.0.2.9000

@jennybc
Copy link
Member

jennybc commented Sep 21, 2023

I'm about to merge #297 because it's definitely progress.

But just want to note this is also connected to #246. When that gets addressed, make sure to test +.

With #297:

library(glue)

str(as_glue("foo") + character())
#>  'glue' chr "foo"

str(glue("foo{x}", x = character()))
#>  'glue' chr(0)

With glue v1.6.2:

library(glue)

str(as_glue("foo") + character())
#>  'glue' chr(0)

str(glue("foo{x}", x = character()))
#>  'glue' chr(0)

jennybc added a commit that referenced this issue Sep 21, 2023
* Improve addition method

Fixes #286

* Add test re: double interpolation

---------

Co-authored-by: Jennifer (Jenny) Bryan <jenny.f.bryan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants