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

Stricter alignemnt detection for unequal number of tokens on different lines #903

Merged
merged 6 commits into from
Jan 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/touchstone-comment.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ jobs:
${{ github.event.workflow_run.event == 'pull_request' &&
github.event.workflow_run.conclusion == 'success' }}
steps:
- uses: lorenzwalthert/touchstone/actions/comment@add-pat
- uses: lorenzwalthert/touchstone/actions/comment@main
with:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
2 changes: 1 addition & 1 deletion .github/workflows/touchstone-receive.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ jobs:
RSPM: ${{ matrix.config.rspm }}
GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: lorenzwalthert/touchstone/actions/receive@add-pat
- uses: lorenzwalthert/touchstone/actions/receive@main
with:
cache-version: 1
benchmarking_repo: ${{ matrix.config.benchmarking_repo }}
Expand Down
3 changes: 3 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@
* `Warning: Unknown or uninitialised column:` was fixed (#885).
* Unaligned expressions with quoted key (e.g. `c("x" = 2)`) are now correctly
detected (#881).
* function calls with unequal number of token on different lines are no longer
deemed aligned if there are arbitrary spaces around the tokens on the lines
with most tokens (#902).
* ensure a trailing blank line also if the input is cached (#867).
* Preserve trailing blank line in roxygen examples to simplify concatenation of
examples (#880).
Expand Down
44 changes: 32 additions & 12 deletions R/detect-alignment.R
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ token_is_on_aligned_line <- function(pd_flat) {
if (any(starting_with_comma)) {
return(FALSE)
}
pd_is_multi_line <- map_lgl(pd_by_line, ~ any(.x$multi_line > 0L, na.rm = TRUE))
pd_is_multi_line <- map_lgl(
pd_by_line,
~ any(.x$multi_line > 0L, na.rm = TRUE)
)
if (any(pd_is_multi_line)) {
return(FALSE)
}
Expand All @@ -98,9 +101,9 @@ token_is_on_aligned_line <- function(pd_flat) {
alignment_ensure_trailing_comma()
# now, pd only contains arguments separated by values, ideal for iterating
# over columns.

n_cols <- map_int(pd_by_line, ~ sum(.x$token == "','"))
previous_line <- 0
current_col <- 0
start_eval <- ifelse(alignment_col1_all_named(pd_by_line), 1, 2)
for (column in seq2(1, max(n_cols))) {
by_line <- alignment_serialize_column(pd_by_line, column) %>%
Expand All @@ -109,33 +112,47 @@ token_is_on_aligned_line <- function(pd_flat) {
trimws(which = "right")
# check 1: match by comma
# might have fewer lines in subsequent columns.
current_col <- nchar(by_line)
max_previous_col <- max(current_col)

# first col has no leading ,
current_col <- nchar(by_line) - as.integer(column > 1)
if (column > 1) {
previous_line <- previous_line[intersect(names(previous_line), names(by_line))]
previous_line <- previous_line[
intersect(names(previous_line), names(by_line))
]
# must add previous columns, as first column might not align
current_col <- current_col + previous_line
}

is_aligned <- length(unique(current_col)) == 1L
if (!is_aligned) {
if (!is_aligned || length(current_col) < 2) {
# check 2: left aligned after ,
current_col <- nchar(gsub("^(,[\\s\\t]*)[^ ]*.*$", "\\1", by_line, perl = TRUE))
current_col <- "^(,[\\s\\t]*)[^ ]*.*$" %>%
gsub("\\1", by_line, perl = TRUE) %>%
nchar() %>%
magrittr::subtract(1)

if (column > 1) {
# must add previous columns, as first column might not align
current_col <- current_col + previous_line
current_col <- previous_line + current_col
}
is_aligned <- length(unique(current_col)) == 1L
if (length(current_col) > 1) {
is_aligned <- length(unique(current_col)) == 1L
} else {
is_aligned <- current_col - max_previous_col == 1
current_col <- max_previous_col + current_col
}

if (is_aligned) {
# if left aligned after ,
start_eval <- 2
}
}
if (is_aligned) {
previous_line <- previous_line + nchar(by_line)
previous_line <- current_col
next
}
# check 3: match by = (no extra spaces around it allowed.)
# check 3: match by = (no extra spaces around it allowed)
# match left aligned after =
start_after_eq <- regexpr("= [^ ]", by_line)
names(start_after_eq) <- names(by_line)
Expand All @@ -145,14 +162,17 @@ token_is_on_aligned_line <- function(pd_flat) {
if (length(start_after_eq) == 0) {
return(FALSE)
}
# when match via comma unsuccessful, matching by = must yield at least one =
# when match via , unsuccessful, matching by = must yield at least one =
if (column == 1) {
current_col <- start_after_eq
} else {
current_col <- start_after_eq +
previous_line[intersect(names(previous_line), names(start_after_eq))]
}
is_aligned <- length(unique(current_col)) == 1 && length(start_after_eq) > 1
is_aligned <- all(
length(unique(current_col)) == 1,
length(start_after_eq) > 1
)
if (!is_aligned) {
return(FALSE)
}
Expand Down
16 changes: 16 additions & 0 deletions tests/testthat/alignment/cols-with-one-row-in.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
c(
"x", "z",
"cgjhg", "thi", "z"
)


c(
"x", "z",
"cgjhg", "thi", "z"
)


c(
"x", "y", "z", "m", "n", "o", "p",
"c", "d"
)
73 changes: 73 additions & 0 deletions tests/testthat/alignment/cols-with-one-row-in_tree

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 16 additions & 0 deletions tests/testthat/alignment/cols-with-one-row-out.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
c(
"x", "z",
"cgjhg", "thi", "z"
)


c(
"x", "z",
"cgjhg", "thi", "z"
)


c(
"x", "y", "z", "m", "n", "o", "p",
"c", "d"
)
2 changes: 1 addition & 1 deletion tests/testthat/alignment/named-in.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ call(
# algorithm: aligned. human: aligned.
call(
x = 1,
xy = 2, n = 33, z = "333"
xy = 2, n = 33, z = "333"
)

# algorithm: aligned. human: aligned.
Expand Down
4 changes: 2 additions & 2 deletions tests/testthat/alignment/named-in_tree

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion tests/testthat/alignment/named-out.R
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ call(
# algorithm: aligned. human: aligned.
call(
x = 1,
xy = 2, n = 33, z = "333"
xy = 2, n = 33, z = "333"
)

# algorithm: aligned. human: aligned.
Expand Down