Skip to content
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

Why is KNITR_WIDTH a global option and not a package option? #597

Closed
krlmlr opened this issue Sep 5, 2013 · 10 comments
Closed

Why is KNITR_WIDTH a global option and not a package option? #597

krlmlr opened this issue Sep 5, 2013 · 10 comments
Labels
next Issues/PRs considered for the next release
Milestone

Comments

@krlmlr
Copy link
Contributor

krlmlr commented Sep 5, 2013

I have found KNITR_WIDTH through code inspection, after I have noticed that knitr resets option(width=75). Very useful indeed.

I am willing to contribute documentation, but then I'm unsure where to put it. Perhaps in options.md? But then a new section, "Global options" would have to be introduced.

This leads to the question: Why is KNITR_WIDTH a global option in the first place? Wouldn't it be more consistent to use a package option here? Besides KNITR_WIDTH, only config.pandoc seems to be queried using the getOption call.

@yihui
Copy link
Owner

yihui commented Sep 6, 2013

Because opts_knit requires knitr to be loaded, whereas options() can be used anywhere (e.g. in ~/.Rprofile) since it is a base R function. I kind of regret the invention of opts_knit now. Perhaps I should have used options(knitr.foo) to set the package option foo from day one; although now all the package options can be read from options() in that way.

KNITR_WIDTH might be a bad name as well... perhaps it should be knitr.width

I have not documented it because I have not made the decision yet.

@krlmlr
Copy link
Contributor Author

krlmlr commented Sep 6, 2013

A transition to global options instead of opts_knit still should be possible, by forwarding the calls to opts_knit to the global options interface. May I suggest maintaining a distinction between chunk and package option using a different prefix, say, knitr.chunk. vs. knitr.?

I haven't found a corresponding issue in the issue tracker. Also, we should not forget documenting knitr.width once the transition is done.

@yihui yihui closed this as completed in 1f8dfd2 Sep 28, 2013
yihui added a commit that referenced this issue Sep 28, 2013
@yihui
Copy link
Owner

yihui commented Sep 28, 2013

I do not plan to do that to opts_chunk. Now KNITR_WIDTH has been renamed to knitr.width, and documented.

@yihui yihui reopened this Nov 20, 2013
@yihui
Copy link
Owner

yihui commented Nov 20, 2013

On second thought, I think it makes sense to be able to set chunk options via something like options(knitr.chunk.foo) for opts_chunk$set(foo), but then I will have to break what I did before again, because for package options, I cannot use options(knitr.foo). I need to change the convention to options(knitr.package.foo). I wonder if there will be many users yelling at me for this change...

@krlmlr
Copy link
Contributor Author

krlmlr commented Nov 20, 2013

The breaking change shouldn't be necessary unless you have a package option that starts with chunk.. ?

@yihui
Copy link
Owner

yihui commented Nov 20, 2013

That is right, and I do not have a package option starting with chunk., but for the sake of a more consistent naming convention, knitr.chunk.foo and knitr.package.foo are more favorable.

@krlmlr
Copy link
Contributor Author

krlmlr commented Nov 20, 2013

I think knitr.chunk and knitr.package are too long for a prefix anyway. Why not use knitr and knitrc prefixes? No compatibility hassles + (more or less) clean naming convention.

@yihui
Copy link
Owner

yihui commented Nov 20, 2013

The main purpose of this is to allow users to change the default package options and chunk options in ~/.Rprofile, which only needs to be done once. Most users probably do not need to do this at all. So I do not think a long prefix is a big concern.

@yihui
Copy link
Owner

yihui commented Nov 20, 2013

I'll keep backward compatibility for a while so users can move from options(knitr.foo) to options(knitr.package.foo)

@yihui yihui closed this as completed in 18e30a2 Nov 20, 2013
yihui added a commit that referenced this issue Oct 12, 2016
@github-actions
Copy link

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
next Issues/PRs considered for the next release
Projects
None yet
Development

No branches or pull requests

2 participants