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

Only consider R code as code to format #313

Merged
merged 4 commits into from Feb 18, 2018

Conversation

@lorenzwalthert
Copy link
Collaborator

@lorenzwalthert lorenzwalthert commented Jan 4, 2018

Closes #312. @krlmlr should we use a package option to allow specifying the regex pattern for the engine (L69)? I.e. if someone creates a custom engine that uses R (as we did for the tidyverse.org post), this code will be ignored for styling with this patch.

@lorenzwalthert lorenzwalthert requested a review from krlmlr Jan 4, 2018
@codecov-io
Copy link

@codecov-io codecov-io commented Jan 4, 2018

Codecov Report

Merging #313 into master will increase coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #313      +/-   ##
=========================================
+ Coverage   91.22%   91.3%   +0.07%     
=========================================
  Files          30      30              
  Lines        1379    1391      +12     
=========================================
+ Hits         1258    1270      +12     
  Misses        121     121
Impacted Files Coverage Δ
R/transform-code.R 94.87% <100%> (+2.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3de7946...87bebd4. Read the comment docs.

@lorenzwalthert
Copy link
Collaborator Author

@lorenzwalthert lorenzwalthert commented Jan 15, 2018

@krlmlr what do you think regarding the package option?

@lorenzwalthert
Copy link
Collaborator Author

@lorenzwalthert lorenzwalthert commented Jan 29, 2018

Ok, I added the engine pattern as a package option.

@krlmlr
Copy link
Member

@krlmlr krlmlr commented Feb 4, 2018

Sorry, I missed that. I don't think this should be a package option, but adding more arguments to style_pkg() and style_file() feels awkward. Can we refactor the function that decides which engines are supported, so that this function just returns a constant for now?

@lorenzwalthert
Copy link
Collaborator Author

@lorenzwalthert lorenzwalthert commented Feb 9, 2018

Ok, I reset hard and implemented it the way I understand you suggested it. What do you think?

krlmlr
krlmlr approved these changes Feb 10, 2018
Copy link
Member

@krlmlr krlmlr left a comment

Looks good!

@lorenzwalthert lorenzwalthert merged commit 961ac86 into r-lib:master Feb 18, 2018
4 checks passed
@lorenzwalthert lorenzwalthert deleted the fun-dec branch Mar 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants