-
Notifications
You must be signed in to change notification settings - Fork 334
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
Fix for horizontal scrolling in code blocks #1448
Fix for horizontal scrolling in code blocks #1448
Conversation
@@ -174,7 +174,7 @@ build_reference <- function(pkg = ".", | |||
old_dir <- setwd(path(pkg$dst_path, "reference")) | |||
on.exit(setwd(old_dir), add = TRUE) | |||
|
|||
old_opt <- options(width = 80) | |||
old_opt <- options(width = 10000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we leave the width as is? I think this better captures what people see in a console.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually leaving it at 80 will automatically wrap long lines which means that the css will not take effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That’s not entirely true as many functions don’t respect width.
I don’t think we want to simulate and incredibly wide console.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the default print
function for data.frames respects width
which leads to sometimes ugly output in the code block because the data.frame is split. keeping width at 80 renders these changes obsolete.
Also thanks for the feedback, love your work on the r-ecosystem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that, but I don't think it's a good idea to force people to scroll vertically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe exposing it as an argument would be the best way, as this decision must be left to the package developer as to how the output shows in the docs. If said flag is set to TRUE
then the width would be set to max.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sorry but I don't agree — I think it's better for the output to match what you're likely to see in a console, which is not going to be 10,000 characters wide.
I implemented the changes I've put in this issue #1447. (fixes #1447)