-
-
Notifications
You must be signed in to change notification settings - Fork 51
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
Magrittr wrap #68
Magrittr wrap #68
Conversation
…rittr.newline argument in tidy_source.Rd, so entered manually. Tried to wrap usage of tidy_source to less than 60 chars
Thanks! This looks pretty cool, but I probably need to try really hard to find time to review it carefully. Before I do that, I just have a quick question: is this feature safe to character strings that happen to contain the |
Thank you for taking a look, no worries, this is a hobby project for me. To answer your question, the string example you gave wouldn't wrap. Not because I have the regex to exclude pipes in comments, but because my rule is to wrap if there are two or more pipes. But if it is like:
Then it'd wrap like:
[Excuse my %$% language! (jk)....] But you just gave me the thought - what if two or more pipes like %>% or %$% are within a comment or roxygen import? Like:
or In these particular cases unfortunately it would still wrap. Right now I have |
… unit tests" This reverts commit 49689eb.
… for magrittr.newline argument in tidy_source.Rd, so entered manually. Tried to wrap usage of tidy_source to less than 60 chars" This reverts commit 34f07c5.
This reverts commit e497134.
This reverts commit f34adc7.
This reverts commit ce7a3aa.
first, change `%>%` to `%> %` in mask_comments(), i.e., add a number of spaces before the closing %, so that deparse() can break the line after %; then remove the extra spaces in unmask_source()
Conflicts: R/tidy.R R/utils.R
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just simplified the implementation (it's a hack that you probably don't want to understand :). It doesn't support the special case in your previous implementation: when the expression contains only one pipe, the line is still wrapped. Please let me know if you have objections. Thanks a lot!
Hi Yihui,
Hope you are well. I added a new feature to address magrittr line wrapping (issues #54, #62). The unit tests run, and I added couple more.
Most of it was writing the function magrittr_wrap_lines in utils.R, and adding it into the tidy_source function
Appreciate any feedback you may have