-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Making configuration more flexible #457
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
Changes from all commits
6059883
1d22a9a
8d7970b
c056b5c
b74c2c1
ddb0834
cee3296
1743f2a
18c725e
1b5137c
d37821c
3aa6436
c25c5d7
c777913
238dfb7
5eff572
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
use clap::{App, ArgMatches, SubCommand}; | ||
use std::path::PathBuf; | ||
use clap::{ArgMatches, SubCommand, App}; | ||
use mdbook::MDBook; | ||
use mdbook::errors::Result; | ||
use {get_book_dir, open}; | ||
|
@@ -15,10 +16,6 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { | |
.arg_from_usage( | ||
"--no-create 'Will not create non-existent files linked from SUMMARY.md'", | ||
) | ||
.arg_from_usage( | ||
"--curly-quotes 'Convert straight quotes to curly quotes, except for those \ | ||
that occur in code blocks and code spans'", | ||
) | ||
.arg_from_usage( | ||
"[dir] 'A directory for your book{n}(Defaults to Current Directory \ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is the "curly quote" option removed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I removed it because something like that belongs more in the config file than as a command line argument. My thoughts were that options to alter the generated output and which usually stay the same belong in the config file, while options that alter runtime behaviour (e.g. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That sounds reasonable to me, but I am not sure about the intent behind the |
||
when omitted)'", | ||
|
@@ -28,21 +25,16 @@ pub fn make_subcommand<'a, 'b>() -> App<'a, 'b> { | |
// Build command implementation | ||
pub fn execute(args: &ArgMatches) -> Result<()> { | ||
let book_dir = get_book_dir(args); | ||
let book = MDBook::new(&book_dir).read_config()?; | ||
let mut book = MDBook::new(&book_dir).read_config()?; | ||
|
||
let mut book = match args.value_of("dest-dir") { | ||
Some(dest_dir) => book.with_destination(dest_dir), | ||
None => book, | ||
}; | ||
if let Some(dest_dir) = args.value_of("dest-dir") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. .. did you move the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah. I was thinking a renderer would be told to put its stuff in the That way renderers are "sandboxed" and should only be touching stuff in the directory they're told to write output to. #409 means renderers are being given an entirely in-memory representation of the book, so these two combined would mean we're a lot more decoupled from the file system and the environment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kk |
||
book.config.book.build_dir = PathBuf::from(dest_dir); | ||
} | ||
|
||
if args.is_present("no-create") { | ||
book.create_missing = false; | ||
} | ||
|
||
if args.is_present("curly-quotes") { | ||
book = book.with_curly_quotes(true); | ||
} | ||
|
||
book.build()?; | ||
|
||
if args.is_present("open") { | ||
|
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.
You moved the
build-dir
into the common configuration. Hmmm. If I have multiple output generators, I cannot put them into different directorys. I think the other way was better. What are the arguments for doing it this way?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.
My thoughts were that (in the long term) you would have a single
build/
directory where all renderers would put their rendered output. During the rendering stage a particular renderer is simply told the directory they should put things in, instead of each renderer doing their own thing and deciding it themselves.That way
mdbook
would ensure you have a standardized directory layout looking something like this:With the special case being that if you're only using one renderer,
mdbook
will put tell a renderer to put its stuff directly in the root (avoiding the redundant extra level of nesting frombuild/html/
).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.
ok, that sounds fine :)