Replace element with origin in PointerMove

[mjzffr]

Okay, who's the lucky reviewer who wants to help me learn Rust? @jgraham r?

Good: This compiles.
Bad: I haven't been able to test it because I can't compile geckodriver after updating the webdriver dependency to point to my local changes. I think maybe the two are out of sync in many ways.

Anyway, I appreciate the opportunity to write some Rust, and I realize this PR may still need a lot of work.

---

This change is 

[andreastt]

This is very good if this is your first shot at Rust!

[andreastt]

You can dereference *body here to avoid the &Json::String references below.

[mjzffr]

Done.

[andreastt]

Same here with regards to dereferencing *self.

[mjzffr]

Is there any way to clean up String(ref x) and the &*x.to_string() that follows? I'll admit that I resorted to various combinations of ref, & and * until the compiler was happy. Will have to go and read how those actually work at some point.

[jgraham]

Reviewed 1 of 1 files at r1.
Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

---

src/command.rs, line 1513 at r1 (raw file):

Previously, mjzffr (Maja Frydrychowicz) wrote…

Done.

FWIW I prefer using the &Json style since it avoids taking ownership of the value, which can cause problems with the borrow checker (although I have a feeling that maybe the compiler was improved so this case isn't as problematic as it used to be).

---

src/command.rs, line 1515 at r1 (raw file):

Previously, mjzffr (Maja Frydrychowicz) wrote…

Is there any way to clean up String(ref x) and the &*x.to_string() that follows? I'll admit that I resorted to various combinations of ref, & and * until the compiler was happy. Will have to go and read how those actually work at some point.

Right, so.

Your original variable is a &Json::String which contains a String. A string literal is a type &'static str. I don't know if you read up on how string types work in Rust, but it's a notorious source of confusion. In brief a String is an owned type so it's saying "this variable owns a string". String is therefore mutable and you can use it to build larger strings like e.g. a StringBuilder in Java.

An &str is a pointer to a string slice that's owned by someone else. A string literal is effectively owned by the program and has the corresponding 'static lifetime.

When you do the match, writing &Json::String(x) would give away ownership of the String, so that won't pass the borrow checker. So we need &Json::String(ref x). That gives us an &String, which we need to convert into a &str. The trick at this point is that the deref operator * will convert a String to a str, so &* on a String gives an &str, which is what we need. However in this case we have an &String, so we need to write &**x to end up with the type we want.

Does that help at all? I hope it walked the line between patronising and unhelpful…

---

Comments from Reviewable

[jgraham]

Generally I think this looks really good.

---

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

---

Comments from Reviewable

[mjzffr]

Review status: 0 of 1 files reviewed at latest revision, 3 unresolved discussions.

---

src/command.rs, line 1515 at r1 (raw file):

Previously, jgraham wrote…

Right, so.

Your original variable is a &Json::String which contains a String. A string literal is a type &'static str. I don't know if you read up on how string types work in Rust, but it's a notorious source of confusion. In brief a String is an owned type so it's saying "this variable owns a string". String is therefore mutable and you can use it to build larger strings like e.g. a StringBuilder in Java.

An &str is a pointer to a string slice that's owned by someone else. A string literal is effectively owned by the program and has the corresponding 'static lifetime.

When you do the match, writing &Json::String(x) would give away ownership of the String, so that won't pass the borrow checker. So we need &Json::String(ref x). That gives us an &String, which we need to convert into a &str. The trick at this point is that the deref operator * will convert a String to a str, so &* on a String gives an &str, which is what we need. However in this case we have an &String, so we need to write &**x to end up with the type we want.

Does that help at all? I hope it walked the line between patronising and unhelpful…

Yep, that helps. Thanks!

---

src/command.rs, line 1531 at r1 (raw file):

Previously, andreastt (Andreas Tolfsen) wrote…

Same here with regards to dereferencing *self.

Done.

---

Comments from Reviewable