robertdfrench / git-pr

Pull requests without Git(Hub|Lab)
GNU General Public License v3.0
0 stars 1 forks source link

Investigate simplifying fake_git pattern matching #13

Closed calebwherry closed 3 years ago

calebwherry commented 3 years ago

With the fake git mocks, we want simple binaries with the least number of dependencies. So, for CLI options that are dead simple, we don't want to use CLAP (like we do in the actual product binaries). We ran into an issue with matching on Option that required some ugly looking code:

     let first_arg = env::args().nth(1);
     match first_arg {
         None => exit(1),
         Some(val) => {
             if val == "--version" {
                 println!("fake_git version 1");
             } else {
                 exit(1);
             }
         }
     };

For some reason, we could not match using something like Some("--version"). Should be doable and much cleaner to do it this... but how?

calebwherry commented 3 years ago

@robertdfrench Looks like it is known limitation: https://stackoverflow.com/a/48034667

If we use var.as_deref(), that works. Lots of things I don't quite understand about strings here but it works so going to do that.

     let first_arg = env::args().nth(1);
     match first_arg.as_deref() {
         None => exit(1),
         Some("--version") => println!("fake_git version 1"),
         Some(_) => exit(1)
     };
robertdfrench commented 3 years ago

Okay, so that's a new one on me as well. It looks anything that implements Deref has to specify a "Target" type that indicates what kind of thing it dereferences to. In this case, the docs seem to imply that String dereferences to str: https://doc.rust-lang.org/std/string/struct.String.html#impl-Deref

I don't even remotely understand that, because I had been under the impression that str was a reference type for String. But moving past that... it looks like that, since any type implementing the Deref specifies what its associated Target type is, then Option's as_deref method has enough information to convert the value type into its associated target. Or something. I'm lost.

I'm glad you found this, I never would have been able to work it out from first principals.