-
Notifications
You must be signed in to change notification settings - Fork 165
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
Add an option to specify the output directory #104
Changes from 6 commits
c745ce6
dd3fef9
d79bccb
d720eaf
5aa0c49
fbcfd7a
228658a
c2dc448
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 |
---|---|---|
|
@@ -363,7 +363,7 @@ struct ProcessingSession { | |
/// This is the virtual "CWD" that our filesystem accesses use. It is the | ||
/// dirname of `primary_input_path`, or an empty path (i.e., corresponding | ||
/// to the CWD if `primary_input_path` is None. | ||
fs_root: PathBuf, | ||
#[allow(unused)] fs_root: PathBuf, | ||
|
||
/// This is the name of the format file to use. TeX has to open it by name | ||
/// internally, so it has to be String compatible. | ||
|
@@ -382,6 +382,10 @@ struct ProcessingSession { | |
/// engine doesn't know about this path at all. | ||
makefile_output_path: Option<PathBuf>, | ||
|
||
/// This is the path, the processed file will be saved at. It defaults | ||
/// to the path of `primary_input_path` or `.` if STDIN is used. | ||
output_path: PathBuf, | ||
|
||
pass: PassSetting, | ||
output_format: OutputFormat, | ||
tex_rerun_specification: Option<usize>, | ||
|
@@ -426,6 +430,7 @@ impl ProcessingSession { | |
|
||
let mut io_builder = CliIoBuilder::default(); | ||
let primary_input_path; | ||
let output_path; | ||
let fs_root; | ||
let tex_input_stem; | ||
|
||
|
@@ -457,6 +462,22 @@ impl ProcessingSession { | |
} | ||
} | ||
|
||
if let Some(dir) = args.value_of("outdir") { | ||
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. This should use value_of_os to get an OsString from the argument. |
||
|
||
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. Please remove this blank line. |
||
output_path = { | ||
let tmp = PathBuf::from(dir); | ||
match tmp.extension() { | ||
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 don't think this code should special-case the circumstance where the argument path has an extension. It's not common but sometimes directory names have extensions like files, and in those cases you'll get surprising behavior because the program will alter the path that you pass it. |
||
Some(_) => { | ||
tt_warning!(status, "file extension in `-o/--outdir`; only the directory will be used"); | ||
tmp.parent().unwrap().to_path_buf() | ||
} | ||
None => tmp, | ||
}}; | ||
if !output_path.exists() { return Err(ErrorKind::Msg(format!("output directory \"{}\" does not exist", output_path.display())).into());} | ||
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. Please don't condense the bodies of |
||
} else { | ||
output_path = fs_root.clone().to_path_buf() | ||
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. Isn't this a double clone? Just 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. Just 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 see, |
||
} | ||
|
||
let mut aux_path = Path::new(tex_input_stem).to_owned(); | ||
aux_path.set_extension("aux"); | ||
let mut xdv_path = aux_path.clone(); | ||
|
@@ -501,6 +522,7 @@ impl ProcessingSession { | |
tex_pdf_path: pdf_path.into_os_string(), | ||
output_format: output_format, | ||
makefile_output_path: makefile_output_path, | ||
output_path: output_path, | ||
tex_rerun_specification: reruns, | ||
keep_intermediates: args.is_present("keep_intermediates"), | ||
keep_logs: args.is_present("keep_logs"), | ||
|
@@ -692,8 +714,8 @@ impl ProcessingSession { | |
continue; | ||
} | ||
|
||
let mut real_path = self.fs_root.clone(); | ||
real_path.push(name); | ||
let real_path = self.output_path.join(name); | ||
|
||
|
||
status.note_highlighted("Writing ", &real_path.to_string_lossy(), &format!(" ({} bytes)", contents.len())); | ||
|
||
|
@@ -1052,6 +1074,11 @@ fn main() { | |
.help("How much chatter to print when running.") | ||
.possible_values(&["default", "minimal"]) | ||
.default_value("default")) | ||
.arg(Arg::with_name("outdir") | ||
.long("outdir") | ||
.short("o") | ||
.value_name("OUTDIR") | ||
.help("The output directory. Default: Same like INPUT.")) | ||
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. Please format the default description as it appears in the other arguments; something like
|
||
.arg(Arg::with_name("INPUT") | ||
.help("The file to process.") | ||
.required(true) | ||
|
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.
Is
#[allow(unused)]
necessary?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.
I put this in because now it is unused. But maybe it will be needed later?
Else there would be a warning while compiling.
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.
Oh, I see, I misunderstood, I thought the
fs_root
below is this one. I think that it is better to remove it. It's easy to add it back if needed.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.
Okay, I will do this