r-polars icon indicating copy to clipboard operation
r-polars copied to clipboard

Set linter and auto formatter

Open eitsupi opened this issue 1 year ago • 23 comments

I tried styler.equals, but unfortunately, this currently seems to do little more than modify the assign operator. So perhaps styler.equals::style_pkg() should be executed after styler::style_pkg().

eitsupi avatar May 13 '23 13:05 eitsupi

I also ran regular styler first and then styler.equals in this Rstudio add-in.

https://github.com/sorhawell/styler.equals/blob/main/R/saveAndStylerEqualsAddin.R

sorhawell avatar May 13 '23 15:05 sorhawell

Finally seeing this conversation. Based on what is in styler I'm unsure why styler.equals is failing to style packages. Glad you have found out issues with it, any ideas on solutions welcome...

Robinlovelace avatar May 20 '23 22:05 Robinlovelace

Based on what is in styler I'm unsure why styler.equals is failing to style packages.

Thanks for the reply. It seems you've pinpointed the cause in https://github.com/Robinlovelace/styler.equals/issues/3#issuecomment-1556025539.

eitsupi avatar May 21 '23 13:05 eitsupi

@eitsupi I think this can be closed?

etiennebacher avatar Mar 21 '24 08:03 etiennebacher

Just got a notification on this. Seems it can be closed to me. On a different topic, I still didn't get styler.equals to work : ( any ideas how I can fix it? Thanks!

Robinlovelace avatar Mar 21 '24 08:03 Robinlovelace

@etiennebacher I don't think Rust and R linters are set up in CI yet.

eitsupi avatar Mar 21 '24 11:03 eitsupi

I didn't understand this was the point of this issue, I thought task format- and the new linter CI were enough

etiennebacher avatar Mar 21 '24 17:03 etiennebacher

Just got a notification on this. Seems it can be closed to me. On a different topic, I still didn't get styler.equals to work : ( any ideas how I can fix it? Thanks!

Maintainer of {styler} here, I can maybe help you if you open/point me to an issue in your repo and explain the problem to me. Actually, I think you could just take the code developed as part of this StackOverflow answer developed with my help in https://github.com/r-lib/styler/issues/1170 if that's ok for the R Polars maintainers (and sorhawell in particular) from a licensing point of view.

lorenzwalthert avatar Apr 05 '24 16:04 lorenzwalthert

Hey @lorenzwalthert I know you have already helped but would you be willing to provide some pointers on the shelved styler.equals package? I sense that could be an upstream fix, not 100% sure though. Thanks and apologies for jumping in on this thread.

Robinlovelace avatar Apr 06 '24 07:04 Robinlovelace

Thanks for the offer, but AFAIK the only reason this issue is open is because we need to setup a CI for styler. Currently we use styler and modify one of its transformers to prefer = over <- (see this script). Locally we just have to run task format-r

etiennebacher avatar Apr 06 '24 08:04 etiennebacher

Heads-up @eitsupi I think this is fixed upstream in {styler.equals}. Can you check and confirm it work?

Will look to release to CRAN and put it out there if so!

Robinlovelace avatar Apr 29 '24 10:04 Robinlovelace

@Robinlovelace Thank you for working on that! But as stated above, this repository is already not using styler.equals, so don't worry about me.

eitsupi avatar Apr 29 '24 10:04 eitsupi

I think {styer.equals}, as a packaged solution, is more lightweight to use than to define a style guide in this project (if you want tidyverse style + = over <-). Also, it integrates better with RStudio Addins for styling, pre-commit and other tools. In addition, it can correctly format edge cases the style guide developed here can't, e.g. c(a <- 1).

lorenzwalthert avatar Apr 29 '24 11:04 lorenzwalthert

Thanks for the suggestion. I will take a look when I have time.

eitsupi avatar Apr 29 '24 11:04 eitsupi

I was not concerned with the use of = in the first place, and did not want to deviate from the default setting of styler. I was just respecting that because @sorhawell was preferring that.

But now that @sorhawell is not doing the development, I would prefer if I could simplify things by moving away from the use of =.

@etiennebacher Any thoughts?

eitsupi avatar Apr 29 '24 12:04 eitsupi

I don't have a preference between <- and = and in the end I don't think choosing one or the other matters. There are some cases where <- works and = doesn't, e.g if( a <- 1), but I don't like this kind of code as I find it less readable (and I never had to review code with this kind of pattern in this package). = is used in other (widely) used packages so this is not a problem.

We have something that works, I don't think we should spend too much time on this. That said, if you want to change, I have nothing against it.

etiennebacher avatar Apr 29 '24 12:04 etiennebacher

Since I usually work on VS Code using the languageserver package (based on styler) as a formatter, auto-formatting with commands in the editor will use <- due to the default setting of styler (it will also replace any existing =).

I then use the script to replace them with = later, which is not a big hassle, but it is definitely a hassle.

eitsupi avatar Apr 29 '24 12:04 eitsupi

Last comment from my side here: VS Code is a good example for integration with other tools. Now that a packaged style guide exists with {styler.equals} that does what you want (if you still want 😄), you can simply

options(languageserver.formatting_style = function(options) {
    styler.equals::equals_style(scope = "indention", indent_by = options$tabSize)
})

as per the docs of the language server.

lorenzwalthert avatar Apr 29 '24 15:04 lorenzwalthert

Awesome!

Robinlovelace avatar Apr 29 '24 15:04 Robinlovelace

Last comment from my side here: VS Code is a good example for integration with other tools. Now that a packaged style guide exists with {styler.equals} that does what you want (if you still want 😄), you can simply

Thanks for pointing that, looks great! I will try that.

eitsupi avatar Apr 30 '24 08:04 eitsupi

I got a new job and small kids and will most likely not be contributing to any open source for two years or so.

All power to the active developers ! :) = is dead, long live <-

My secret next open source goal is to introduce <-as assignment operator in a python project.

sorhawell avatar Jul 04 '24 19:07 sorhawell

I think we can close this for now then (also speaking with 1 soon to be 2 lottle ones, good luck with it all and thanks for getting the ball rolling on this and supporting the project as it evolves)!

Robinlovelace avatar Jul 04 '24 20:07 Robinlovelace

Thanks for looking into this. I will try to set up a CI with the work on the next branch (maybe pre-commit?), which is still relatively small.

eitsupi avatar Jul 05 '24 02:07 eitsupi