-
Notifications
You must be signed in to change notification settings - Fork 73
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
Enforcing a 80-character line length rule? #247
Comments
Thanks for having consulted open issues first. #118 is an edge case as far as indention goes and not quite what you are referring to. The task of splitting lines of code with more than 80 characters is non-trivial as it is not a-priori clear at which position lines should be split. I think this may also not follow a general and abstract rule (as the other styling rules) but is best done manually and decided on a case-to-case basis. Therefore, this feature is currently no supported in styler and will probably no be so in the near future. We suggest to use the packages lintr to detect lines with more than 80 characters and rearrange manually. Edit: I think we aim to support this in the long run. |
I'm working on (finally!) releasing utf8, this means we could release styler soon. After this I think we could start thinking about that 80-character rule, because editing code from "wide" to "long" takes quite a lot of time (measured in seconds, but still). I'd like to suggest to do it at the level of function calls and arithmetic expressions first:
Maybe start with only function calls -- if we add a set of rules after all other rules (because we need to be sure about the width), and then do only reindention of function call arguments? |
Thanks for the reply! I agree that it's not easy to implement such a rule. @krlmlr 's suggestion looks good to me. |
So do you @krlmlr suggest to take the 80 character limit from the (raw) unstyled expression, right? |
No, that would be after styling, when we know the final width -- we'd just add linebreaks but not change much otherwise. |
Issues that can't be resolved soonish will be closed and labelled "Status: Postponed". |
Reference: #65. |
@lorenzwalthert , I've implemented a version of this, by adding a loop to
I've committed the implementation to a fork (4017b39). It's not pull-request-ready, and I would appreciate your input as to whether it's worth developing into something that is. |
Thanks @krivit for looking into it. To be honest, I first need to think about what I had in mind earlier this year about implementing that feature. I think there were some obstacles I can't quite remember as I was reflecting on different approaches. As it looks to me, your approach seems to work (maybe need to handle cases where indention >= 80 so line break won't help at all and we will be stuck in infinite loop) but I am not sure if there is a better way of doing it. Anyways, I first have to fully understand what you do and maybe also giving it a fresh try myself - at least conceptually, so we can figure out the best way of implementing that feature. As soon as we have reached a high-level consensus about how to go about it, I think we can start moving your branch towards that if you are willing to. I could give you some feedback on your code right away but I think I first want to think about the larger picture before we optimise the details. Also, I would like to only implement line breaks for function calls first as @krlmlr suggested in his comment. Is that ok for you? |
Thanks for reopening the issue. The plan sounds fine. The way I see it, any implementation will be inherently inelegant, for two reasons:
So, any algorithm will be some form of trial-and-error. Here are some additional issues I can think of, in addition to those you had mentioned:
|
@krivit can you open a PR anyways already with the WIP so we don't forget about it? |
@lorenzwalthert, done. |
This commit resolves Bioconductor#57 by updating the `handleMessage()` calls after the formatting check fails. Instead of mentioning `formatR`, these messages now refer to: * `styler`, * `biocthis`, * the `BiocCheck` vignette. Furthermore, the `BiocCheck` vignette section on formatting still mentions `formatR`, but it now also mentions in more detail: * `styler`: how to install and run it * `biocthis`: as a companion to `styler` with code how to run it (but not install it since it's optional and the install instructions will hopefully change soon once this package is submitted/reviewed). * a quick overview of the differences between `formatR` and `styler`, though this is mostly done by highlighting current limitations of `formatR`. * how RStudio Desktop's `Reformat code` button (under the magic wand) can help break long lines which is something that `styler` cannot do as discussed in more detail at r-lib/styler#247. `formatR` can do this, but personally, the fact that it broke valid R code in some of my packages made me trust its output less. The NEWS and DESCRIPTION files have been updated accordingly.
Hi folks, just wondering what the status is on this issue? It would be a helpful addition :-) Thanks. |
I am not currently working on that. We first need to outline conceptually what the implementation (or a first version) will look like. I think what I outlined in #247 (comment) is a good start. |
Ok, thanks! |
But I admit, the growing number of 👍 in the issue is motivating 😄 |
Change global and targets to `character`, see r-lib/styler#247, it is rather hard to to styling for long lines. Signed-off-by: Liang Zhang <psychelzh@outlook.com>
Hi guys, one year after the last question... 😉 is there any chance that this would be implemented in the near future? I've been using styler quite actively for a couple of months now and I think this is the only major missing piece. |
I know this is high up on the wish list for many users but I anticipate this to be quite complex and time consuming to implement. Given other priorities in my life as well as in {styler}, I won't build this soon unfortunately. I hope you can understand this. |
For knitting PDF documents Carlos Luis Rivera posted a very useful answer on my StackOverflow post, which might be helpful for some people. It requires including the following to your header: ---
title: "R Notebook"
output: pdf_document
header-includes:
- |
```{=latex}
\usepackage{fvextra}
\DefineVerbatimEnvironment{Highlighting}{Verbatim}{
breaksymbolleft={},
showspaces = false,
showtabs = false,
breaklines,
commandchars=\\\{\}
}
```
---
```{r}
knitr::opts_chunk$set(tidy="styler")
``` |
This commit resolves #57 by updating the `handleMessage()` calls after the formatting check fails. Instead of mentioning `formatR`, these messages now refer to: * `styler`, * `biocthis`, * the `BiocCheck` vignette. Furthermore, the `BiocCheck` vignette section on formatting still mentions `formatR`, but it now also mentions in more detail: * `styler`: how to install and run it * `biocthis`: as a companion to `styler` with code how to run it (but not install it since it's optional and the install instructions will hopefully change soon once this package is submitted/reviewed). * a quick overview of the differences between `formatR` and `styler`, though this is mostly done by highlighting current limitations of `formatR`. * how RStudio Desktop's `Reformat code` button (under the magic wand) can help break long lines which is something that `styler` cannot do as discussed in more detail at r-lib/styler#247. `formatR` can do this, but personally, the fact that it broke valid R code in some of my packages made me trust its output less. The NEWS and DESCRIPTION files have been updated accordingly. Former-commit-id: 5604071
I use styler to format R code. Lintr gives me warning on lines on 80 characters line limit. However it seems styler has no option to specify max number of characters per line. I'd rather enforce the line width automatically instead of manually. How do R users enforce line width limit in Rstudio or in vim?
|
Hi everyone, I opened the issue #1192, and I just realized it's a duplicate of this. Should I close my issue, or not? Thanks. |
Is it possible to enforce a rule to limit code to 80 character per line?
styler
currently doesn't seem to do that. The closest issue I can find is #118, but it's not clear if that option is available.The text was updated successfully, but these errors were encountered: