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

Investigate simplifying fake_git pattern matching #13

Closed
calebwherry opened this issue Oct 11, 2021 · 2 comments
Closed

Investigate simplifying fake_git pattern matching #13

calebwherry opened this issue Oct 11, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@calebwherry
Copy link
Collaborator

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
Copy link
Collaborator Author

calebwherry commented Oct 11, 2021

@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)
     };

@calebwherry calebwherry self-assigned this Oct 11, 2021
@calebwherry calebwherry added this to the Git PR v1.0.0 milestone Oct 11, 2021
@calebwherry calebwherry added the enhancement New feature or request label Oct 11, 2021
@robertdfrench
Copy link
Owner

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.

@calebwherry calebwherry added this to In progress in GitPr Kanban Oct 12, 2021
GitPr Kanban automation moved this from In progress to Done Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

2 participants