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

Operators starting a line in an expression with parentheses #255

Closed
krlmlr opened this issue Oct 24, 2017 · 13 comments
Closed

Operators starting a line in an expression with parentheses #255

krlmlr opened this issue Oct 24, 2017 · 13 comments

Comments

@krlmlr
Copy link
Member

krlmlr commented Oct 24, 2017

styler::style_text("function() {\n (a\n||b)\n}", strict = FALSE, scope = "line_breaks")
#> function() {
#>   (a
#>   || b)
#> }

This is rather low priority, just mentioning it for completeness.

@lorenzwalthert
Copy link
Collaborator

So do you mean the operator should be moved one line up or?

@krlmlr
Copy link
Member Author

krlmlr commented Oct 25, 2017

I'm not sure, maybe if we had a rule in tidystyle that forbids starting a line with a binary operator?

@lorenzwalthert
Copy link
Collaborator

I think there is no rule. Should I propose one or is is a sufficiently rare case?

@krlmlr
Copy link
Member Author

krlmlr commented Oct 27, 2017

Good idea.

@lorenzwalthert
Copy link
Collaborator

Issue filed.

@lorenzwalthert
Copy link
Collaborator

In tidyverse/style#45, it was not considered to be worth to mention in the style guide since such lengthy conditions are considered to be undesirable in the first place.

@lorenzwalthert
Copy link
Collaborator

So I suggest to either move the infix operators one line up anyways (for scope >= "line_breaks") or just leave them as is.

@krlmlr
Copy link
Member Author

krlmlr commented Nov 3, 2017

Can we leave them unchanged and just add a level of indention?

@lorenzwalthert
Copy link
Collaborator

Ok, I think we could, it's just we have never done it that way so far I think. So we'd have to add one level of indention only to the token || and leave everything else unchanged since indention is already added (but not active) for all tokens after ||.

@krlmlr
Copy link
Member Author

krlmlr commented Nov 4, 2017

But then it seems easy to also add indention for the operator token itself, I understand it will be inactive in most cases?

@krlmlr
Copy link
Member Author

krlmlr commented Nov 4, 2017

And this probably generalizes to other operators?

So, if we are processing (a + b), the + and the b gets indention, which is then activated if there's a newline before either?

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Nov 5, 2017

No, the triggering token itself does (currently) not get indention, only the ones following it. So in your example (to be precise in (a + \nb)), only b gets indented because the closing brace is not in the same nest. It was done that way because braces trigger indention, but they should not be indented. I.e, you don't want

{
  2
}

To become

  {
  2
}

@lorenzwalthert
Copy link
Collaborator

However, your case seems a special case. Need to look into it further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants