r-lib / styler

Non-invasive pretty printing of R code
https://styler.r-lib.org
Other
703 stars 70 forks source link

Overriding tidyverse function arg style #1184

Closed JosiahParry closed 3 months ago

JosiahParry commented 3 months ago

I'd pose this in a discussion but there are not any for this repo!

The tidyverse style guide says:

In both cases the trailing ) and leading { should go on the same line as the last argument.

However, it the closing ) and opening { on the same line as the last argument give me the ick! 😜 Is there a way to modify styler to put the closing paren and opening bracket on their own line when there are many function arguments?

I use codegrip to format my function arguments. However, I have recently configured my environment to use styler to format on save. This one styling, though, i'd like to override.

reverse_geocode <- function(
    locations,
    crs = sf::st_crs(locations),
    ...,
    lang_code = NULL,
    feature_type = NULL,
    for_storage = FALSE,
    location_type = c("rooftop", "street"),
    preferred_label_values = c("postalCity", "localCity"),
    geocoder = default_geocoder(),
    token = arc_token(),
    .progress = TRUE
) {
IndrajeetPatil commented 3 months ago

I agree that this should be the default, but, unfortunately, since the underlying style guide specifies this behaviour, we need to stick to it.

As for codegrip, there is a PR to bring it in line with the tidyverse style guide, so it might also start behaving the same way as {styler}.

That said, {grkstyle} defaults to the behaviour you desire, so you might want to check it out.

JosiahParry commented 3 months ago

Gotya, so are you saying that styler is so opinionated that it will not permit modification outside of the tidyverse style guide?

Edit: grkstyle does not adhere to balanced delimiters

IndrajeetPatil commented 3 months ago

It's not {styler} that is opinionated, it's the style guide πŸ™ƒ

But {styler} does allow further customizations: https://styler.r-lib.org/articles/customizing_styler.html

TBH, I'd much rather the style guide change its recommendation here.

JosiahParry commented 3 months ago

{styler} is enforcing the style guide, thus opinionated! πŸ™ˆ Are the stylers documented? It's tough to figure out which one it might be.

Say I've figured out how to customize my styler, how do I utilize it as a global standard? For example with lintr, there is a .lintr file. The customizing-style, third party integration, and distribute custom styles vignettes do not describe how to make a custom styler the default when working with styler.

For context, I'm using VS Code to format on save and not manually styling a file from the R console. Ideally it can be modified in the Rprofile or other environment location

JosiahParry commented 3 months ago

I believe this may be possible in https://github.com/r-lib/lintr/pull/1411 by @MichaelChirico but unclear how πŸ€”

lorenzwalthert commented 3 months ago

The customizing-style, third party integration, and distribute custom styles vignettes do not describe how to make a custom styler the default when working with styler.

Two options:

  1. You just call {styler} with specifying transformers argument with the style guide you export from your package.

  2. You don't use {styler} directly anymore if you create your own style guide based on the styler infra, you just call your own pkg where style_file() and friends have a different default for transformers . E.g. see this README.

    library(styler.noncomments)
    style_text('1 # comment') # this default to transformers = nocomments_style
lorenzwalthert commented 3 months ago

a directory specific configuration for styler is an open issue: #319

JosiahParry commented 3 months ago

@lorenzwalthert I have no intention of creating and distributing my own R package for modifying a single style rule. It would ideal to be able to change or remove one rule in the transformers list and have that be used. Perhaps it can be set via options() in a user-level .Rprofile?

lorenzwalthert commented 3 months ago

For VS code specifically, see https://github.com/REditorSupport/languageserver?tab=readme-ov-file#customizing-formatting-style.

You can modify a rule inline, e.g. in the above mentioned link, but I think for caching reasons, you should also set name and version of the style guide to avoid caching issues as described in https://github.com/r-lib/styler/issues/1170#issuecomment-1849400384.

  ...
  transformers$style_guide_name = "rpolars_style"
  transformers$style_guide_version = "0.1.0"
  ...
lorenzwalthert commented 3 months ago

I think it seems like the style guide might change in the future or at least allow both options since Lionel supports your point of view in https://github.com/lionel-/codegrip/pull/16, but for this to happen, I think as @IndrajeetPatil said, it needs a formal decision upstream. Depending on what happens there, we could change the behaviour or expose an argument in tidyverse_style() to configure the behaviour.

lorenzwalthert commented 3 months ago

@lorenzwalthert I have no intention of creating and distributing my own R package for modifying a single style rule.

Then I am afraid you have to adhere to the style guide, or open an issue in tidyverse/style.

JosiahParry commented 3 months ago

Heard.

Just to be crystal clear, the maintainers of {styler} have no intention of making it possible to simply modify styler similar to lintr's ability to modify them via options e.g.

options(lintr.line_length_lintr = NULL)

and instead, require users to create their own R package for this behavior?

lorenzwalthert commented 3 months ago

I might be able to show you how you can modify styler to achieve what you want, but unless you create your own style guide, the option won’t be as portable as a simple one liner in a config file like for {lintr}.

lorenzwalthert commented 3 months ago

Intentions we have to make configuration

  1. of the style guide function to use
  2. Of the arguments to pass to the style guide

easier to access through a config file or similar as noted in #319.

But time and resources and priorities are a different topic 😜

To achieve what you wanted in your original post, we would first have to expose an argument to the default style guide tidverse_style to allow configuring styler in a way that the style guide currently clearly states what the compliant style looks like, leaving everything else non-compliant.

JosiahParry commented 3 months ago

Closing in favor of #319

lionel- commented 3 months ago

FWIW I think we'd be supportive of a style guide PR to bring it in line with codegrip behaviour. We've originally aligned our guide to the google C++ one, but after working with Rust and Typescript where the balanced style seem to be the norm we think it's best to align with these languages.