Skip to content

Commit

Permalink
Update snapshots with edits made to src that need to be checked
Browse files Browse the repository at this point in the history
  • Loading branch information
@carols10cents
carols10cents committed Jul 15, 2022
1 parent 9232dec commit 83788ff
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 348 deletions.
136 changes: 13 additions & 123 deletions nostarch/chapter12.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,27 +68,6 @@ a file to search in, like so:
$ cargo run -- searchstring example-filename.txt
```

<!---
Depending on platform, the above might be written as
```
$ cargo run -- searchstring example-filename.txt
```
This is mentioned in the cargo run help:
cargo-run
Run a binary or example of the local package
USAGE:
cargo run [OPTIONS] [--] [args]...
I know it's optional, but I think it's a bit more failsafe to separate cargo
and its arguments from your app and your app's arguments.
/JT --->
<!-- Good call, I've updated this here and throughout where relevant. /Carol -->

Right now, the program generated by `cargo new` cannot process arguments we
give it. Some existing libraries on *https://crates.io/* can help with writing
a program that accepts command line arguments, but because you’re just learning
Expand Down Expand Up @@ -132,21 +111,6 @@ adding `use std::env::args` and then calling the function with just `args`,
because `args` might easily be mistaken for a function that’s defined in the
current module.

<!---
"it’s conventional to bring the parent module into scope rather than the
function"
I'm not sure if we have a strong standard. The first thing that came to mind
was "how does rustfmt handle it?" and it doesn't have any preferred format.
Same for clippy.
I'd say we could show them how to do it, but I wouldn't say anything about
convention.
/JT --->
<!-- Fair, I changed "it's conventional" to "we've chosen". /Carol -->

> ### The `args` Function and Invalid Unicode
>
> Note that `std::env::args` will panic if any argument contains invalid
Expand All @@ -170,13 +134,19 @@ first with no arguments and then with two arguments:
```
$ cargo run
--snip--
["target/debug/minigrep"]
[src/main.rs:5] args = [
"target/debug/minigrep",
]
```

```
$ cargo run -- needle haystack
--snip--
["target/debug/minigrep", "needle", "haystack"]
[src/main.rs:5] args = [
"target/debug/minigrep",
"needle",
"haystack",
]
```

Notice that the first value in the vector is `"target/debug/minigrep"`, which
Expand Down Expand Up @@ -292,14 +262,6 @@ statement: we need `std::fs` to handle files [1].
In `main`, the new statement `fs::read_to_string` takes the `file_path`, opens
that file, and returns a `std::io::Result<String>` of the file’s contents [2].

<!---
The above returns `std::io::Result<String>`. Calling it `Result<String>` is a
bit ambiguous and may confuse the reader.
/JT --->
<!-- Totally right, I've fixed! /Carol -->

After that, we again add a temporary `println!` statement that prints the value
of `contents` after the file is read, so we can check that the program is
working so far [3].
Expand Down Expand Up @@ -578,21 +540,6 @@ fn main() {
}
```

<!---
Not sure how nitty I'm being but worth a mention:
Cloning in a constructor feels a bit awkward, as the clones will take
additional memory which could exceed the system memory and cause a panic. We
may want to promote a "constructors don't fail" way of thinking where possible.
For that, we'd need to move to using two `String` params for the `new`
function, which also feels a bit more Rust-y way of doing it.
/JT --->
<!-- We fix this in Chapter 13, and I haven't heard people saying "constructors
don't fail" before... I can see how that would be important in some contexts,
but I would hesitate to push that generally. /Carol -->

Listing 12-7: Changing `parse_config` into `Config::new`

We’ve updated `main` where we were calling `parse_config` to instead call
Expand Down Expand Up @@ -700,19 +647,6 @@ impl Config {
}
```

<!---
Similar to above, I think having infallible constructors are a bit more Rust-y.
For times where you need to construct and that construction can potentially
fail, we should use a different name than `new` to key people in that they
aren't getting a constructor but instead they're maybe getting the type they
want (or maybe an error).
/JT --->
<!-- Ok, you've convinced me to change the name from `new` to `build` even
though I don't think the "constructors should be infallible" philospohpy is
universal. /Carol -->

Listing 12-9: Returning a `Result` from `Config::build`

Our `build` function returns a `Result` with a `Config` instance in the success
Expand Down Expand Up @@ -916,7 +850,6 @@ fn main() {
if let Err(e) = run(config) {
println!("Application error: {e}");
process::exit(1);
}
}
Expand Down Expand Up @@ -1145,7 +1078,7 @@ Now let’s run the test:
$ cargo test
Compiling minigrep v0.1.0 (file:///projects/minigrep)
Finished test [unoptimized + debuginfo] target(s) in 0.97s
Running unittests (target/debug/deps/minigrep-9cd200e5fac0fc94)
Running unittests src/lib.rs (target/debug/deps/minigrep-9cd200e5fac0fc94)
running 1 test
test tests::one_result ... FAILED
Expand Down Expand Up @@ -1479,34 +1412,6 @@ from the `run` function. First, we’ll add a configuration option to the
Adding this field will cause compiler errors because we aren’t initializing
this field anywhere yet:

<!-- JT: I decided to change the field name and the environment variable to be
called `ignore_case` to avoid some double-negative confusion detailed in this
issue: https://github.com/rust-lang/book/issues/1898 I'd love your thoughts
especially on the names and logic throughout this section! Thank you!!
/Carol -->
<!---
I've left a few comments. I think the name reads okay.
I think my recurring thought here (which you'll see in a few places), is if
we want to make a constructor, to do more work outside of the constructor so
that the constructor itself is infallible.
Or, we could rename the function to something like `build_config` or something.
Then it feels a bit more like "well sure, it's possible it could fail to build
if I give it something wrong".
Not sure if we have space here, but in a real-world version of your example we'd
probably use the builder pattern, so you could optionally grab the value
from the environment or not. When you're writing tests, things that find their
settings from the env tend to be a pain as you have to incorporate that into
the testing. But if you can use the builder pattern, you can use another way
of configuring the value that is more test-friendly.
/JT --->
<!-- While I do love the builder pattern, that would be a bigger change than I
want to make right now to explain it thoroughly. /Carol -->

Filename: src/lib.rs

```
Expand All @@ -1517,9 +1422,9 @@ pub struct Config {
}
```

We added the `ignore_case` field that holds a Boolean. Next, we need the
`run` function to check the `ignore_case` field’s value and use that to
decide whether to call the `search` function or the `search_case_insensitive`
We added the `ignore_case` field that holds a Boolean. Next, we need the `run`
function to check the `ignore_case` field’s value and use that to decide
whether to call the `search` function or the `search_case_insensitive`
function, as shown in Listing 12-22. This still won’t compile yet.

Filename: src/lib.rs
Expand Down Expand Up @@ -1578,20 +1483,6 @@ impl Config {
}
```

<!---
Same comment on this one, too. We can largely avoid confusion here I think by
just not naming it `new`.
Taking in the args as a slice of Strings also feels less Rust-y than having
names for the two parameters that will be part of the Config.
/JT --->
<!-- I've changed the name from `new` to `build`, and we change the way this
gets arguments somewhat in chapter 13. The *real* Rusty way would be to use an
argument parser crate like clap... and I don't want to use external crates in
the book any more than the one usage of rand in chapter 2 :) /Carol -->

Listing 12-23: Checking for any value in an environment variable named
`IGNORE_CASE`

Expand Down Expand Up @@ -1699,7 +1590,7 @@ stream so we can still see error messages on the screen even if we redirect the
standard output stream to a file. Our program is not currently well-behaved:
we’re about to see that it saves the error message output to a file instead!

To demonstrate this behavior, we’ll run the program with `>` and the file_path,
To demonstrate this behavior, we’ll run the program with `>` and the file path,
*output.txt*, that we want to redirect the standard output stream to. We won’t
pass any arguments, which should cause an error:

Expand Down Expand Up @@ -1742,7 +1633,6 @@ fn main() {
if let Err(e) = minigrep::run(config) {
eprintln!("Application error: {e}");
process::exit(1);
}
}
Expand Down
Loading

0 comments on commit 83788ff

Please sign in to comment.