Skip to content

ignore a NULL output_dir when cleaning empty directory #859

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.

Sign up for GitHub

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

Jump to bottom
Merged
yihui merged 6 commits into from
Feb 9, 2020
Merged

ignore a NULL output_dir when cleaning empty directory #859

yihui merged 6 commits into from
Feb 9, 2020

Conversation

@cderv
Copy link
Collaborator

@cderv cderv commented Feb 9, 2020

This will fix #857

Currently in render_book, when output_dir = ".", it will be set internally to NULL as bookdown will ignore it in several places

bookdown/R/utils.R

Lines 82 to 94 in 581bb63

output_dirname = function(dir, config = load_config(), create = TRUE) {
if (is.null(dir)) {
dir2 = config[['output_dir']]
if (!is.null(dir2)) dir = dir2
}
if (is.null(dir)) dir = '_book'
if (length(dir)) {
if (create) dir_create(dir)
# ignore dir that is just the current working directory
if (same_path(dir, getwd())) dir = NULL
}
dir
}

Currently, clean_empty_dir throws an error when output_dir is NULL

bookdown/R/render.R

Lines 78 to 79 in 581bb63

output_dir = output_dirname(output_dir, config)
on.exit(clean_empty_dir(output_dir), add = TRUE)

With this PR, clean_empty_dir will also ignore output_dir = NULLas the current working directory will not be empty and we don't want to remove it anyway.

From my understanding, dir could be NULL for clean_empty_dir only in this special case, as otherwise output_dirname() will have resolved differently.

This PR also adds tests for this utils function.

cderv added 4 commits February 9, 2020 09:12
@cderv
Ignore a NULL directory when trying to clean
33d495d 
output_dir is set to NULL internally when provided explicitly by user to current directory, and in this case it is not empty and should not be cleaned (rstudio#857)
@cderv
add test about clean_empty_dir
73976c6
@cderv
add a NEWS bullet
75370d6
@cderv
small typo
5a62bcb
@cderv cderv requested a review from yihui February 9, 2020 08:44
yihui
yihui approved these changes Feb 9, 2020
View reviewed changes
Copy link
Contributor

@yihui yihui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thank you!

@yihui yihui merged commit 3fa700f into rstudio:master Feb 9, 2020
@cderv cderv deleted the fix-clean-current-dir branch February 10, 2020 06:31
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@yihui yihui yihui approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Using root as output directory gives an error

2 participants

@cderv @yihui