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.

Sign up for GitHub

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

Jump to bottom

Indentation + trailing whitespace behaviour changed between versions 1.5.0 and 1.5.1 #247

Closed
paleolimbot opened this issue Dec 13, 2021 · 3 comments
Closed

Indentation + trailing whitespace behaviour changed between versions 1.5.0 and 1.5.1 #247

paleolimbot opened this issue Dec 13, 2021 · 3 comments

Comments

@paleolimbot
Copy link
Member

When upgrading to glue 1.5.1, we noticed a difference in the handling of indentation because we use glue in the arrow R package for code generation. It's a fairly easy workaround (see apache/arrow#11936) but I couldn't find anything in the merged PRs list or changelog that indicates that this was intentional. A few reprexes that I hope are helpful! It seems like the handling of trailing whitespace changed.

# remotes::install_github("tidyverse/glue")
glue::glue("\n  text\n  with\n  indentation\n  \n")
#>   text
#>   with
#>   indentation
#> 
glue::glue("\n  text\n  with\n  indentation")
#> text
#> with
#> indentation
# remotes::install_github("cran/glue@1.5.0")
glue::glue("\n  text\n  with\n  indentation\n  \n")
#> text
#> with
#> indentation
glue::glue("\n  text\n  with\n  indentation")
#> text
#> with
#> indentation
@jennybc
Copy link
Member

jennybc commented Dec 14, 2021

Happened with 7172e09.

@paleolimbot
Copy link
Member Author

Thank you!

@jennybc
Copy link
Member

jennybc commented Dec 15, 2021

I plan to do a release with this and #236. Remains to be seen if that can happen before the holidays.

@jonkeane jonkeane mentioned this issue Mar 31, 2022
Closed
jonkeane added a commit to apache/arrow that referenced this issue Mar 31, 2022
@jonkeane
MINOR: [R] Avoid {glue}'s whitespace trimming
a1f32fa 
For a while now glue has been removing the tabs before: (this might be from tidyverse/glue@8369f9a which was to change [a different thing we noticed with whitespace stripping](tidyverse/glue#247)).

Regardless, the fix is easy enough (don't include new lines around the line we are glueing)

Before the change, the format of the relevant lines of arrowExports.cpp are:
```
static const R_CallMethodDef CallEntries[] = {
{ "_arrow_available", (DL_FUNC)& _arrow_available, 0 },
{ "_dataset_available", (DL_FUNC)& _dataset_available, 0 },
{ "_engine_available", (DL_FUNC)& _engine_available, 0 },
{ "_parquet_available", (DL_FUNC)& _parquet_available, 0 },
{ "_s3_available", (DL_FUNC)& _s3_available, 0 },
{ "_json_available", (DL_FUNC)& _json_available, 0 },
{ "_arrow_test_SET_STRING_ELT", (DL_FUNC) &_arrow_test_SET_STRING_ELT, 1},
```

When they should be:
```
static const R_CallMethodDef CallEntries[] = {
	{ "_arrow_available", (DL_FUNC)& _arrow_available, 0 },
	{ "_dataset_available", (DL_FUNC)& _dataset_available, 0 },
	{ "_engine_available", (DL_FUNC)& _engine_available, 0 },
	{ "_parquet_available", (DL_FUNC)& _parquet_available, 0 },
	{ "_s3_available", (DL_FUNC)& _s3_available, 0 },
	{ "_json_available", (DL_FUNC)& _json_available, 0 },
	{ "_arrow_test_SET_STRING_ELT", (DL_FUNC) &_arrow_test_SET_STRING_ELT, 1},
```

This change restores the indentation we expect there (so the file won't change on rebuilds)

Closes #12770 from jonkeane/whitespace

Authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
jcralmeida pushed a commit to rafael-telles/arrow that referenced this issue Apr 19, 2022
@jonkeane @jcralmeida
MINOR: [R] Avoid {glue}'s whitespace trimming
af9bbe0 
For a while now glue has been removing the tabs before: (this might be from tidyverse/glue@8369f9a which was to change [a different thing we noticed with whitespace stripping](tidyverse/glue#247)).

Regardless, the fix is easy enough (don't include new lines around the line we are glueing)

Before the change, the format of the relevant lines of arrowExports.cpp are:
```
static const R_CallMethodDef CallEntries[] = {
{ "_arrow_available", (DL_FUNC)& _arrow_available, 0 },
{ "_dataset_available", (DL_FUNC)& _dataset_available, 0 },
{ "_engine_available", (DL_FUNC)& _engine_available, 0 },
{ "_parquet_available", (DL_FUNC)& _parquet_available, 0 },
{ "_s3_available", (DL_FUNC)& _s3_available, 0 },
{ "_json_available", (DL_FUNC)& _json_available, 0 },
{ "_arrow_test_SET_STRING_ELT", (DL_FUNC) &_arrow_test_SET_STRING_ELT, 1},
```

When they should be:
```
static const R_CallMethodDef CallEntries[] = {
	{ "_arrow_available", (DL_FUNC)& _arrow_available, 0 },
	{ "_dataset_available", (DL_FUNC)& _dataset_available, 0 },
	{ "_engine_available", (DL_FUNC)& _engine_available, 0 },
	{ "_parquet_available", (DL_FUNC)& _parquet_available, 0 },
	{ "_s3_available", (DL_FUNC)& _s3_available, 0 },
	{ "_json_available", (DL_FUNC)& _json_available, 0 },
	{ "_arrow_test_SET_STRING_ELT", (DL_FUNC) &_arrow_test_SET_STRING_ELT, 1},
```

This change restores the indentation we expect there (so the file won't change on rebuilds)

Closes apache#12770 from jonkeane/whitespace

Authored-by: Jonathan Keane <jkeane@gmail.com>
Signed-off-by: Jonathan Keane <jkeane@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jennybc @paleolimbot