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

Code formatting in usage section #820

Closed
lorenzwalthert opened this issue Nov 4, 2018 · 10 comments
Closed

Code formatting in usage section #820

lorenzwalthert opened this issue Nov 4, 2018 · 10 comments
Labels

Comments

@lorenzwalthert
Copy link
Contributor

@lorenzwalthert lorenzwalthert commented Nov 4, 2018

Rendered help files for multi-line function calls have a Usage section that look like the one (taken from for dplyr::join()):

# S3 method for class 'tbl_df'
inner_join(x, y, by = NULL, copy = FALSE,
  suffix = c(".x", ".y"), ...,
  na_matches = pkgconfig::get_config("dplyr::na_matches"))

I think developers should potentially have control over how their Usage section is printed. In particular, to make it compliant with their style guide of choice. The tidyverse style guide would suggest to put the closing brace on a new line and only put first few unnamed arguments on the first line:

# S3 method for class 'tbl_df'
inner_join(x, y,
  by = NULL, copy = FALSE,
  suffix = c(".x", ".y"), ...,
  na_matches = pkgconfig::get_config("dplyr::na_matches")
)

I wondered about how that could be implemented in roxygen2 - with or without styler. We have discussed a styler implementation here: r-lib/styler#415.

@hadley
Copy link
Member

@hadley hadley commented Apr 19, 2019

Yes, this would be great!

@lorenzwalthert
Copy link
Contributor Author

@lorenzwalthert lorenzwalthert commented Apr 21, 2019

I played around a bit but since I did not know the roxygen code base before, I don't feel confident enough to make a PR with the current code. Maybe @krlmlr do you want to jump in? How should we give the user control about how to exactly style the code? We could let the user specify a styler style guide (defaulting to the tidyverse style guide) or if it should also allow other source code formatter than styler, a function that takes text and returns styled text, defaulting to styler::style_text. Could we do that via the RoxygenNote field in the DESCRIPTION file?

@hadley
Copy link
Member

@hadley hadley commented Apr 22, 2019

That is too much control — at most, we should have a switch for the old formatting vs the new formatting (one argument per line), and we should only do that if we think there is some strong reason to believe that people might prefer the old formatting.

@hadley
Copy link
Member

@hadley hadley commented Apr 22, 2019

Unfortunately the code for usage wrapping is scattered in a few places. Currently, the wrapping is done in https://github.com/klutometis/roxygen/blob/master/R/rd.R#L277-L281, but by that point all we have is a string, so it's too late to implement something more specific. Instead, we'd need to tweak args_string() and usage_arts().

The code is complicated by the fact that the usage statement is not pure R code, but also needs to include Rd formatting for \method().

@lorenzwalthert
Copy link
Contributor Author

@lorenzwalthert lorenzwalthert commented Apr 22, 2019

This summarizes my struggles well...

@lorenzwalthert
Copy link
Contributor Author

@lorenzwalthert lorenzwalthert commented Apr 22, 2019

[...] new formatting (one argument per line) [...].

Note that one argument per line is no enforced by styler. What is enforced is that if a call is multi-line and contains named arguments, the line is broken before the first named argument. For details, see tidyverse/style#39. In the style guide, it says in section 2.6:

You may also place several arguments on the same line if they are closely related to each other, e.g., strings in calls to paste() or stop(). When building strings, where possible match one line of code to one line of output.

For this reason, we don't automatically break lines and styler would also be quite a bit more invasive if we'd force line breaks after named arguments and I am not sure we want that. The above paragraph was added in tidyverse/style#52.

@hadley
Copy link
Member

@hadley hadley commented Apr 22, 2019

The constraints on formatting usage are different to styling arbitrary code, especially since all the arguments are named.

@lorenzwalthert
Copy link
Contributor Author

@lorenzwalthert lorenzwalthert commented Apr 22, 2019

Ok, fair point. Once we can make roxygen working with styler::tidyverse_style, we can add a rule to break lines on after named arguments within the scope of roxygen.

@hadley
Copy link
Member

@hadley hadley commented Jul 23, 2019

I don't think roxygen2 will ever be able to use styler because roxygen2 has to wrap things that are a mix of Rd tags and R code. So here I think we just need to have a heuristic: if they fit, all the arguments go on one line, otherwise, each argument gets its own line.

@lorenzwalthert
Copy link
Contributor Author

@lorenzwalthert lorenzwalthert commented Sep 17, 2019

Great, thanks for making this work 🎉.

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

Successfully merging a pull request may close this issue.

None yet
2 participants