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

Styler and %>%: attempt to apply non-function #421

Closed
dirkschumacher opened this issue Sep 10, 2018 · 8 comments
Closed

Styler and %>%: attempt to apply non-function #421

dirkschumacher opened this issue Sep 10, 2018 · 8 comments

Comments

@dirkschumacher
Copy link

styler::style_text("1 %>% 1")
#> 1 %>% 1()

I guess it would make sense to at least check for primitive types on the right hand side before adding brackets to the AST.

Otherwise this feature creates runtime errors.

I am not sure if it is a good idea to always assume the custom operator is a magrittr pipe. I for instance have a use-case where it isn't.

@lorenzwalthert
Copy link
Collaborator

Thanks. Well I guess you are right that we could check if the RHS is a symbol. However, it might be a bit irritating if your example does not return an error or warning but instead just returns the code as is. It only seems to make sense if you don't use the magrittr definition of %>%, which is rarely the case if you use the tidyverse style guide. Anyways, I am happy to check for symbol from a theoretical perspective.

@lorenzwalthert
Copy link
Collaborator

Not sure I understand why it creates runtime errors in general. With the magrittr definition of %>%, the input already will cause runtime errors, no?

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Sep 10, 2018

I agree the infix SPECIAL infix operator should not be assumed to be the magrittr pipe. However, I think it is not. It's just assumed that %>% is the magrittr pipe. Can you post your example?

@lorenzwalthert
Copy link
Collaborator

lorenzwalthert commented Sep 10, 2018

Here is what I get with f8b991e:

styler::style_text("1 %x% 1 ;1 %x% y; 1 %>% x; 1 %s% 1", scope = "tokens")
#> 1 %x% 1
#> 1 %x% y
#> 1 %>% x()
#> 1 %s% 1

Created on 2018-09-10 by the reprex
package
(v0.2.0).

@dirkschumacher
Copy link
Author

Whoa, you are fast thanks a lot! 👍 👍

I should have written could create runtime errors if %>% is not the magrittr pipe and yes, it only concerend the %>% special operator.

We have a usecase where we use special operators for comparisons in a certain context that differ from the standard compare functions in terms of NA and floating point handling (and some other details).

@lorenzwalthert
Copy link
Collaborator

So do you mean you don't want styler to add braces to function calls for %>% in general? I don't think we are going to support that for the tidyverse style guide.
You have the following options:

styler::style_text("a %>% b", scope = "line_breaks")
#> a %>% b

Created on 2018-09-11 by the reprex package (v0.2.0).

@msberends
Copy link

the url in the previous post should be:
https://lorenzwalthert.netlify.com/post/customizing-styler-the-quick-way/

@lorenzwalthert
Copy link
Collaborator

Thanks @msberends, I moved all posts to ./post and forgot to alias. Should be ok with https://github.com/lorenzwalthert/blogdown-cactus/commit/db829d1131fd45857cb549f724f3a428d5e873b1.

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

3 participants