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

Better message when mdbook clean is run on clean workspace #1046

Closed
amanjeev opened this issue Oct 3, 2019 · 4 comments · Fixed by #1055
Closed

Better message when mdbook clean is run on clean workspace #1046

amanjeev opened this issue Oct 3, 2019 · 4 comments · Fixed by #1055

Comments

@amanjeev
Copy link
Member

amanjeev commented Oct 3, 2019

Currently, if you run mdbook clean twice, you get the following error:

$ mdbook clean
2019-10-02 20:18:03 [ERROR] (mdbook::utils): Error: Unable to remove the build directory
2019-10-02 20:18:03 [ERROR] (mdbook::utils):    Caused By: No such file or directory (os error 2)

Would it make sense to show a better message to the user when running mdbook clean on an already clean workspace?

If you think this proposal makes sense, I could perhaps do this but I would need suggestions from you. If you think I am missing something and this is not a good idea, please close this issue.

Thank you.

@rnitta
Copy link
Contributor

rnitta commented Oct 3, 2019

How about replacing fs::remove_dir_all with fs::remove_dir_content like here or here?

If it is done, the error due to nonexistence of the build directory will not occur, because it will not be removed once created.
So the error message will make more sense.

@amanjeev
Copy link
Member Author

amanjeev commented Oct 3, 2019

@rnitta Thank you. I would like to know if it is ok to not remove the build directory? If you see, as an example, cargo clean which removes the target directory. What do you think?

@ehuss
Copy link
Contributor

ehuss commented Oct 4, 2019

I think it would be good if it didn't print anything if the output directory doesn't exist, and exited 0. Also, unless you see a reason, I think it would be fine to just delete the directory, too, I don't see a reason to keep an empty directory around. That is, just wrap the fs::remove_dir_all with a check if the path exists.

@amanjeev
Copy link
Member Author

amanjeev commented Oct 4, 2019

@ehuss I think that is also consistent with cargo clean. Will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants