Math token spacing

[lorenzwalthert]

This PR gives granular control over spacing around math tokens, as envisioned in #82 and requested in tidyverse/lubridate#577 (comment).
Implements a new paradigm for transformer functions in the sense that strict is now an argument to
the transformer. This reduces duplication of code, since the only part that set_* and add_* functions for spacing rules are different is that set_* uses pd_flat$spaces[...] <- level wherease add_* uses pd_flat$spaces[...] <- pmax(1, pd_flat$spaces[...].
Functions that follow this paradigm should be named style_* instead of add_* and set_*.
pd_flat is the last argument to style_space_around_math_token() because this allows using purrr::partial() without naming arguments.

[lorenzwalthert]

As a follow-up and for consistency, other spacing rules could also be transformed from set_* and add_* to style_.

[krlmlr]

Thanks. I haven't looked through all of the code, but the results make sense. Maybe we should consider an option that adds explicit parens to complex arithmetic expressions? In this case, the spacing wouldn't matter that much.

[krlmlr]

I wonder if the other way round would be more usual, because of operator precedence:

+1 + +1 - 3 / 23 * 3^4

[lorenzwalthert]

Right, I can adapt the test. The default should remain as in the tidyverse style guide, so all operators have spacing around them I think.

[krlmlr]

Maybe default_math_token_spacing?

[lorenzwalthert]

Not sure whether I understand you correctly. What does it have to do with a default? Everything will be specified unambiguously. We can also drop the specify from the name in order to make it sound a bit more simplistic.

[krlmlr]

Oh, I see. I only saw the use of the function as a default argument. Maybe implement two functions then, math_token_spacing() and default_math_token_spacing(), and have the latter forward to the former?

[lorenzwalthert]

Ok, we can do that.

[lorenzwalthert]

I am not sure whether we need the forwarder. I.e. if we only have one style guide, then that might be fine, but other style guides may have other defaults. So maybe call it tidyverse_math_token_spacing()?

[lorenzwalthert]

We cannot use a function that has the same name as an argument (math_token_spacing()):

tidyverse_style <- function(scope = "tokens",
                            strict = TRUE,
                            indent_by = 2,
                            start_comments_with_one_space = FALSE,
                            math_token_spacing = math_token_spacing()) {
# [...]

Because we get

#> Error: promise already under evaluation: recursive default argument reference or earlier
#> problems?

I suggest to revert to specify_math_token_spacing(), since that is also a verb and functions are supposed to be verbs according to the tidyverse style guide I think.

[lorenzwalthert]

@krlmlr ok? We can keep tidyverse_math_token_spacing() that forwards to specify_math_token_spaching().

[lorenzwalthert]

Yes, maybe we can consider adding braces to complex expressions. However, I am not sure how straightforward that is. Depends a lot on the structure of the nested parse data I think. Maybe other PR or issue?

[lorenzwalthert]

Closes #86.