rstudio / rstudio

RStudio is an integrated development environment (IDE) for R
https://posit.co/products/open-source/rstudio/
Other
4.67k stars 1.09k forks source link

Feature request: format R codes on save #2563

Open yutannihilation opened 6 years ago

yutannihilation commented 6 years ago

Currently, RStudio can automatically formats codes on save a bit:

As styler reached 1.0, how about integrating the feature with RStudio? Or, is it possible to let users to insert "before save hook"?

In my understanding, "format on save" is a very common feature in modern editors/IDEs. For example, VS Code lets users to choose the formatter and when/whether to format the code or not:

https://code.visualstudio.com/docs/editor/codebasics#_formatting

For another example, Emacs doesn't have direct "format on save", but users can define before-save-hook to format codes:

https://github.com/dominikh/go-mode.el/blob/ff87a1e80945d958d9c348ce4d2e65a797b38a4b/go-mode.el#L1151

I also believe "format on save" is useful to soften the frustration from the incompleteness of "format on type" (e.g. #2371).

ronblum commented 6 years ago

@yutannihilation Thank you for the observations and suggestions. We'll consider them as we work on future versions of RStudio.

kevinushey commented 6 years ago

We've thought a little bit about allowing user-defined save hooks as well. The main difficulty is handling save hooks that misbehave -- e.g. we want to make sure they can easily be interrupted, and that we can report / disable save hooks that don't behave well.

I definitely agree this would be a powerful / useful feature; however, it's one that we'll have to think about carefully if we approach it.

yutannihilation commented 6 years ago

Thanks for the response and sharing the concerns! I understand things are more complicated than I thought.

we can report / disable save hooks that don't behave well.

It sounds like that we need something like "Addin Store," where RStudio have control on addins :) It's great if it becomes a reality.

breckuh commented 6 years ago

With Javascript/TypeScript and Python, I can't live without Prettier (https://github.com/prettier/prettier) and black (https://github.com/ambv/black). Formatting work is completely eliminated. Having an ultra reliable opinionated code formatted for R would be terrific.

lorenzwalthert commented 6 years ago

I think if you really want to use this feature you could use the RStudio Addin "style active file" and enable saving after styling via the environment variable and assign the keyboard shortcut Ctrl + S to the Addin. See ?styler::style_active_file() for details (and yes, we should probably have used an R option instead of an environment variable). It won't be a perfect substitution for the save hook (i.e. it won't play nicely with devtools::load_all() and friends) but it could abridge for now.

lorenzwalthert commented 5 years ago

On a related note, there is now a styler pre-commit hook: https://github.com/lorenzwalthert/pre-commit-hooks. Instead of on save, this is on commit, but maybe still worth considering.

kalaschnik commented 4 years ago

Any news on this? Coming from VS Code where users can set formatOnSave and select their desired formater/linter over a variety of programming languages is a super convenient thing. It would be great if RStudio would allow users to set a similar option too; Maybe styler would be a good default here as well: https://github.com/r-lib/styler/issues/658

rjake commented 4 years ago

Just chiming in to upvote this. We're trying to standardize code on my team of analysts and would love to automate the use of styler. Would also like if something could be set in .Rproj

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs, per https://github.com/rstudio/rstudio/wiki/Issue-Grooming. Thank you for your contributions.

kalaschnik commented 3 years ago

further activity bump!

kalaschnik commented 3 years ago

I just discovered Hadley Wickham’s wonderful style guide (https://style.tidyverse.org/)! This is awesome work! Just as concise as ESLint and AirBnB style guides in the JS/Node world. @hadley would love that feature!

cigrainger commented 3 years ago

I'm kind of lost as to why this is so difficult to pull off. Couldn't an addin do this? Right now I'm avoiding using RStudio for two major reasons: vim-mode has laggy escape and there's no format on save. The thing about format on save as opposed to precommit is that readability while working is important. In every other language I work in, and when I work in neovim or vscode, I hit save and everything snaps into place. It means I spend less cognitive load thinking about formatting and more time being productive. It's amazing how much code being formatted as you expect it to be helps with scanning and readability.

davidbudzynski commented 3 years ago

Any updates on implementing this feature?

brianthomaslewis commented 2 years ago

Bumping for additional traffic. Adding my voice to the chorus that this would be a great feature to have included in RStudio.

clemens-zauchner commented 2 years ago

After a long time with black in python it is so hard to come back to RStudio without automatically reformatting code on save.

MattCowgill commented 1 year ago

Adding my voice to the chorus here.

Anecdotally, I see a lot of R users migrating to VS Code from the RStudio IDE. The absence of features like this accelerate the migration.

njtierney commented 1 year ago

Hi there just wanting to add that I would love to see this feature implemented - it would be a really nice addition and like others said, help reduce cognitive load because your code would be nicely styled consistently

zWarMob commented 1 year ago

enters the chat Can I have 1 on-save formatter too please?

It has indeed been an indispensable feature in other languages and IDEs

ssh352 commented 9 months ago

I'd like Rstudio to run "Reformat Code" on save. Rstudio formatter seems better than styler because it can break up long lines.

kailukowiak commented 7 months ago

Chiming in here too. This is a pretty necessary feature in 2024 (and well before that honestly).

joeDespres commented 7 months ago

Chiming in here too. This is a pretty necessary feature in 2024 (and well before that honestly).

💯

kevinushey commented 7 months ago

Thanks for the feedback everyone! We're now targeting this work for the following RStudio release, and so we plan to begin work in the coming months.

lorenzwalthert commented 7 months ago

Great news Kevin, if you consider an implementation with {styler}, please let me know if any questions arise. Having worked on {styler} for 7 years and {precommit} as a spin-off project, I can think of the following relevant things:

Obviously up to you what to do with it, but I just thought I'd offer my two cents. I'd be excited if {styler} was integrated more tightly with RStudio, but I don't know if you consider that out of scope and prefer to just integrate the Format code RStudio command.

ssh352 commented 7 months ago

It would be nice if styler and Rstudio Format code could finally be one tool like Ruff in Python or Rustfmt in Rust.

R community is quite small and we don't need two separate formatters.

davidbudzynski commented 7 months ago

It would be nice if styler and Rstudio Format code could finally be one tool like Ruff in Python or Rustfmt in Rust.

R community is quite small and we don't need two separate formatters.

There are many Python formatters so it's not like Ruff is the only one being used. Rustfmt is part of the language toolchain, so if you wanted something like that it would be the R devs you'd need to speak to, not RStudio or Styler devs

ssh352 commented 7 months ago

There are many Python formatters so it's not like Ruff is the only one being used. Rustfmt is part of the language toolchain, so if you wanted something like that it would be the R devs you'd need to speak to, not RStudio or Styler devs

That's true. Python has way more people working on formatters. But Ruff or black are what people really use nowadays (Ruff is much nicer). If R had only one formatter, I am just guessing: maybe that formatter could have be nicer and more feature complete than both styler and Rstudio formatter now?

lorenzwalthert commented 7 months ago

RStudio's formatter was already there before we started developing {styler}, but we wanted a customisable, pure R implementation and R APIs (RStudio's formatter can only style the currently active file and has no easy other way to access the functionality IIRC). {styler} has now many third-party integrations, some enable styling in Jupyter Notebooks or VS Code or as a pre-commit hook. So there were different goals from the beginning, leading to different use cases. Also, as we can't go back in the past, the question you ask seems not particularly relevant to this issue. Of course, anyone who wants can build a much faster R formatter than {styler} if they build it in a faster language (or leverage existing formatting infrastructure written in another language), which could even be more feature complete. I'd be the first to use it 😀. But I don't see that happening because I don't think that's an important frontier in the R ecosystem at the moment, and I also have no idea how to build a formatter for R in Rust to be honest, seems daunting 😀. Seems much easier to implement config support for {styler} or breaking long lines. Contributions welcome.

kevinushey commented 3 months ago

Going to finally start taking a look at this -- sorry for taking so long. Here's a proposed implementation:

Some questions:

lorenzwalthert commented 3 months ago

Great @kevinushey. To address your points, here's my take:

t-kalinowski commented 3 months ago

Should this be an external program ... or should this be an invocable R function registered in the session?

My vote would be for an external program, so we leave the door open for the community to write a CLI app like ruff or black for R. Also, the R session can already be quite overburdened, I wouldn't clutter the main R process with the linter/formatter too.

Should the formatter run on autosave as well (when enabled)? Or only when the user explicitly saves the file

My vote would be to run only on explicit saves. It's disconcerting if the text changes while you are just looking at it.

How important are "format range" gestures for R?

I think it's moderately important. This is an existing feature with Cmd+Shift+A, so hopefully that won't be removed. Two situation where this is useful:

kevinushey commented 2 months ago

Thanks @lorenzwalthert -- do you have any strong opinions around how RStudio should invoke styler in this case? I'm assuming we'll want to use styler::style_file()?

kevinushey commented 2 months ago

I think it's moderately important. This is an existing feature with Cmd+Shift+A, so hopefully that won't be removed. Two situation where this is useful:

  • The user is contributing to a git repo where the formatter is not used by the original authors. The contributor wants to minimize the diff size, to format only the lines they changed, but not the full file.

  • The user occasionally disagrees with the formatter, and rather than hack the formatter options, they prefer to apply the formatter in a very deliberate manner and then fix-up the code.

If I'm reading this correctly, this is more an argument for allowing customization of the formatter used for the "Reformat Code" command, as opposed to the behavior of a "format on save" feature? (And we should probably use the same formatter both on save, and for reformatting a snippet of code.)

I guess I was wondering -- if we allow for "format on save", should the whole file be formatted? Or just the lines that were edited? If the latter, how should we communicate which lines were edited to the reformatting tool? For example, imagine edits were made both at the start of a file, and at the end of a file -- how should we communicate those ranges?

For posterity, ruff has:

Editor options:
      --range <RANGE>
          When specified, Ruff will try to only format the code in the given range.
          It might be necessary to extend the start backwards or the end forwards, to fully enclose a logical line.
          The `<RANGE>` uses the format `<start_line>:<start_column>-<end_line>:<end_column>`.

          - The line and column numbers are 1 based.
          - The column specifies the nth-unicode codepoint on that line.
          - The end offset is exclusive.
          - The column numbers are optional. You can write `--range=1-2` instead of `--range=1:1-2:1`.
          - The end position is optional. You can write `--range=2` to format the entire document starting from the second line.
          - The start position is optional. You can write `--range=-3` to format the first three lines of the document.

          The option can only be used when formatting a single file. Range formatting of notebooks is unsupported.

It seems like it only supports a single range, as opposed to multiple ranges.

lorenzwalthert commented 2 months ago

Thanks @lorenzwalthert -- do you have any strong opinions around how RStudio should invoke styler in this case? I'm assuming we'll want to use styler::style_file()?

Yes, I suggest styler::style_file()

Is it possible to configure the default format style used by styler via a dotfile, or something similar? Or should we provide UI in RStudio to allow users to select the default style?

Styler does currently not support a configuration file, but it's an open issue: https://github.com/r-lib/styler/issues/319. I actually liked how {renv} does it, and opened an issue there to expose the mechanics for third-party: https://github.com/rstudio/renv/issues/1865 but no answer yet 😀.

However, the style for the RStudio Addin can be customised with the R option styler.addins_style_transformer, which defaults to styler::tidyverse_style(). Hence, you may want to use this as well until we have some other configuration backend. It's a rather flexible way to specify the style (but prone to injection attacks), since you have all the customisation options of tidyverse_style() at your disposal, plus, you can also distribute your own style guide as an R package and reference it in the option and it will work, as long as you installed the package (e.g. oneliner::one_line_style()).

Similarly, what scopes should RStudio select when formatting on save? Should we allow this to be configured via UI in RStudio, or would it be better to configure this externally (e.g. a .styler dotfile; R options; something else?)

Scope can also be specified as part of styler.addins_style_transformer.

lorenzwalthert commented 2 months ago

I guess I was wondering -- if we allow for "format on save", should the whole file be formatted? Or just the lines that were edited? If the latter, how should we communicate which lines were edited to the reformatting tool? For example, imagine edits were made both at the start of a file, and at the end of a file -- how should we communicate those ranges?

The RStudio Addins supports formatting selected code, i.e. that functionality is already there (kind of). But probably that functionality can only be used if {styler} is invoked from within RStudio and not through an external command. One should note that formatting a range is more complicated (especially if start or end line are not the whole line, but just a part of it) and currently not very well unit-tested I think (not sure how addins unit testing works, or how we can capsule it), so before making this available to RStudio users, I guess someone would have to work on that (contributions appreciated).

kevinushey commented 2 months ago

The next daily builds will now have preliminary support for:

Configuration for the active formatter is available in the new "Formatting" tab:

Screenshot 2024-08-02 at 1 54 25 PM

Reformatting on save is controlled in the "Saving" tab:

Screenshot 2024-08-02 at 1 54 36 PM

If you're interested in testing and providing feedback, these builds are available at https://dailies.rstudio.com. Please let us know what you think!

kalaschnik commented 2 months ago

Thanks @kevinushey for working on this! It is always the biggest bummer for me having no format-on-save in RStudio. This will also remove clutter from git diffs and will be a big gain for R's code consistency. Thank you ❤️

lorenzwalthert commented 2 months ago

Hi Kevin, thanks for implementing this, here are my observations and comments.

Screenshot 2024-08-04 at 23 31 00

My observations refer to RStudio-2024.10.0-daily-82 build for macOS.

olivroy commented 1 month ago

Similar to #14835, I think that Format on Save doesn't respect the existing line ending.

It should also probably be a project option? I would love this feature on my own projects, but seems invasive to projects I am collaborating on.

kevinushey commented 1 week ago

The magic wand (picture below) seems to work when I select format on save and auto-saving off and then save manually after some edits.

Thanks; I can fix this up.

auto-saving on won't file the document. This might be desired, but then even manually triggering a saving won't format on save since the latest changes were already saved and there is nothing to save. Is that desired?

This should be fixed; any explicitly-requested saves should trigger formatting of the document.

Reformatting unsaved document does not seem to work.

Can you clarify? Using "Reformat Selection" works for me, even in unsaved documents.

kevinushey commented 1 week ago

I think that Format on Save doesn't respect the existing line ending.

I think this would need to be a feature of styler? Although I guess we could post-process the document after formatting.

lorenzwalthert commented 5 days ago

I am not sure what you mean with line ending, but we had an issue about support for different types of line endings: https://github.com/r-lib/styler/issues/396