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

bail! in render() if specified theme directory does not exist #1791

Merged
merged 2 commits into from
May 2, 2022

Conversation

clement2026
Copy link
Contributor

@clement2026 clement2026 commented Apr 24, 2022

This PR fixes #1756.

With the following config,

#book.toml

[output.html]
theme = "./xxx"

if ./xxx directory does not exist, an error message will be logged:

#log

2022-04-25 02:19:12 [INFO] (mdbook::book): Book building has started
2022-04-25 02:19:12 [INFO] (mdbook::book): Running the html backend
2022-04-25 02:19:12 [ERROR] (mdbook::config): theme dir "./xxx" does not exist
2022-04-25 02:19:12 [INFO] (mdbook::cmd::serve): Serving on: http://localhost:3000
2022-04-25 02:19:12 [INFO] (warp::server): Server::run; addr=127.0.0.1:3000
2022-04-25 02:19:12 [INFO] (warp::server): listening on http://127.0.0.1:3000 
2022-04-25 02:19:12 [ERROR] (mdbook::config): theme dir "./xxx" does not exist
2022-04-25 02:19:12 [INFO] (mdbook::cmd::watch): Listening for changes...

@ehuss
Copy link
Contributor

ehuss commented Apr 25, 2022

Thanks! I think I would expect mdbook to exit with an nonzero status if there is an error. Maybe instead of adding a check to the theme_dir method, it could just check if it exists in the render method, and bail! if it doesn't exist?

@clement2026
Copy link
Contributor Author

Thanks! I think I would expect mdbook to exit with an nonzero status if there is an error. Maybe instead of adding a check to the theme_dir method, it could just check if it exists in the render method, and bail! if it doesn't exist?

Yes, this looks much better. I've changed the content of commit.

With the following config,

#book.toml

[output.html]
theme = "./xxx"

if ./xxx directory does not exist, build will fail:

$ mdbook build
2022-04-26 20:33:37 [INFO] (mdbook::book): Book building has started
2022-04-26 20:33:37 [INFO] (mdbook::book): Running the html backend
2022-04-26 20:33:37 [ERROR] (mdbook::utils): Error: Rendering failed
2022-04-26 20:33:37 [ERROR] (mdbook::utils): 	Caused By: theme dir /Users/clark/projects/site/./xxx does not exist

@clement2026 clement2026 changed the title log error message if specified theme directory does not exist bail! in render() if specified theme directory does not exist Apr 26, 2022
@ehuss
Copy link
Contributor

ehuss commented Apr 27, 2022

Thanks, can you add a test for this? (I know it is a bit trivial, but I think it is good to keep test coverage of error cases.)

@clement2026
Copy link
Contributor Author

Thanks for the reminder! I've added a test function. Since it's my first time to write a test for mdBook, let me know if anything is missing.

Copy link
Contributor

@ehuss ehuss 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, thanks!

There is some risk that this will cause some unexpected errors for some projects, but I don't expect that to affect too many people.

@ehuss ehuss merged commit 5bea831 into rust-lang:master May 2, 2022
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 this pull request may close these issues.

Theme directory not being read
2 participants