Merge into a single crate. Use macros even on unstable.

[SimonSapin]

Breaking changes:

• ns!("") should be written ns!()
• Other ns!(…) macros should be lowercase, unquoted.

Fixes #124
Closes #123

r? @asajeffrey

[asajeffrey]

The move to macros is great! As a coding style issue, are we encouraging the style of ns!(xml) or ns!("xml")?

---

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

---

build.rs, line 70 [r1] (raw file):
Add std::slice and std::mem to the imports list?

---

src/atom/mod.rs, line 308 [r1] (raw file):
I don't see how this fits in a u64: one byte for length + 7 bytes data + 1 bit for the tag = 65 bits. Add a unit test for this?

---

src/lib.rs, line 56 [r1] (raw file):
Are we encouraging ns!("xml") or ns!(xml)?

---

Comments from the review on Reviewable.io

[SimonSapin]

Review status: 15 of 16 files reviewed at latest revision, 3 unresolved discussions.

---

build.rs, line 70 [r1] (raw file):
Done.

---

src/atom/mod.rs, line 308 [r1] (raw file):
The length is stored in the upper 4 bits of the same bytes that has the tag, which is enough since it only goes up to 7: https://github.com/servo/string-cache/blob/0854229fbe/src/atom/mod.rs#L341

Are the tests at https://github.com/servo/string-cache/blob/0854229fbe/src/atom/mod.rs#L465-L466 enough?

---

src/lib.rs, line 56 [r1] (raw file):
The former is not supported anymore. But this choice was fairly arbitrary. If you prefer it I can change the macro.

---

Comments from the review on Reviewable.io

[asajeffrey]

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/atom/mod.rs, line 308 [r1] (raw file):
Oh silly me, the unpacked representation is 65 bits, it then packs down to 64.

Since you're being explicit about packing to u64 I don't think you need a unit test.

---

src/lib.rs, line 56 [r1] (raw file):
Perhaps a conversation on irc to see if anyone has opinions? I slightly prefer ns!("xml") but I don't have a strong opinion.

---

Comments from the review on Reviewable.io

[bors-servo]

☔ The latest upstream changes (presumably #126) made this pull request unmergeable. Please resolve the merge conflicts.

[SimonSapin]

Here is how it works in this last commit and why:

• atom!(…) only accepts quoted strings. That string’s value is exactly the value that the atom will get (through Deref<Target=str>), so it makes sense to use the syntax of &str string literals.
• ns!(…) only accepts unquoted identifiers. The identifier is a namespace prefix like svg that is typically used but it is not the value that ends up in the atom, which is the namespace URL like "http://www.w3.org/2000/svg". Identifiers are used to name values.

This removes alternative/synonymous syntax based on "there should be one way of doing it". (This may cause more churn for updating html5ever and Servo, though.)

[Ms2ger]

It's an identifier, not a string, so I agree with not quoting the argument to ns!().

[asajeffrey]

Reviewed 5 of 5 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from the review on Reviewable.io

[asajeffrey]

@bors-servo r+

[bors-servo]

🔑 Insufficient privileges

[SimonSapin]

@bors-servo r=asajeffrey

[bors-servo]

📌 Commit 4d505d6 has been approved by asajeffrey

[bors-servo]

⌛ Testing commit 4d505d6 with merge 76d6428...

[bors-servo]

☀️ Test successful - travis