rewrite of mime_classifier.rs to use more iterators

[tafia]

Rewrite few parts of the file to use more iterators.

Note that I have no idea what the code is actually doing functionally, I just tried to mimic exactly what was being done. All tests are ok

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @larsbergstrom (or someone else) soon.

[tafia]

I don't really understand the error. I doesn't seem to be linked with my changes (I don't use setuptools).

Error running mach:

    ['test-tidy']

The error occurred in code that was called by the mach command. This is either
a bug in the called code itself or in the way that mach is calling it.

You should consider filing a bug for this issue.

If filing a bug, please include the full output of mach, including this error
message.

The details of the failure are as follows:

ImportError: No module named setuptools

  File "/home/johann/projects/servo/python/servo/testing_commands.py", line 222, in test_tidy
    return tidy.scan()
  File "./python/tidy.py", line 288, in scan
    errors = list(itertools.chain(errors, r_errors))
  File "./python/tidy.py", line 248, in collect_errors_for_files
    for error in check(file_name, contents):
  File "./python/tidy.py", line 133, in check_flake8
    from flake8.main import check_code
  File "./python/dependencies/flake8-2.4.1-py2.py3-none-any.whl/flake8/main.py", line 6, in <module>

[Ms2ger]

components/net/mime_classifier.rs:93: trailing whitespace
components/net/mime_classifier.rs:178: trailing whitespace
components/net/mime_classifier.rs:194: trailing whitespace
components/net/mime_classifier.rs:200: trailing whitespace
components/net/mime_classifier.rs:205: trailing whitespace
components/net/mime_classifier.rs:352: trailing whitespace
components/net/mime_classifier.rs:372: trailing whitespace
components/net/mime_classifier.rs:383: trailing whitespace
components/net/mime_classifier.rs:385: trailing whitespace
components/net/mime_classifier.rs:393: trailing whitespace
components/net/mime_classifier.rs:399: trailing whitespace
components/net/mime_classifier.rs:400: trailing whitespace
components/net/mime_classifier.rs:402: trailing whitespace
components/net/mime_classifier.rs:403: trailing whitespace

[tafia]

How did you get this ? Is there a command to check for all the trailing whitespaces ?

[Ms2ger]

Yes, that's what ./mach test-tidy is supposed to do, but in this case, I just copied from the Travis log.

[tafia]

Ok thanks. Just removed said whitespaces.

I can't run ./mach test-tidy on my side (cf my previous message).
Should I fill a separate issue for that ?

[tafia]

And rebased

[Ms2ger]

Yeah, please do file that.

[bors-servo]

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

[tafia]

rebased again

[bors-servo]

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

[jdm]

Sorry, I started reviewing this and got interrupted. I'll finish it up today.

[jdm]

Thanks for doing this work! I have some comments that I think should make the code more clear for future readers, but I appreciate this effort :)
-S-awaiting-review

---

Review status: 0 of 1 files reviewed at latest revision, 14 unresolved discussions, some commit checks failed.

---

^{components/net/mime_classifier.rs, line 128 [r2] (raw file):}
Why s and m?

---

^{components/net/mime_classifier.rs, line 131 [r2] (raw file):}
nit:

let result = self.clone().zip(...);
if result {
    self.nth(matches.len());
}
result

---

^{components/net/mime_classifier.rs, line 153 [r2] (raw file):}
Does .zip(&self.pattern).zip(&self.mask) work?

---

^{components/net/mime_classifier.rs, line 154 [r2] (raw file):}
all(|((&data, &pattern), &mask)| (data & mask) == (pattern & mask))

---

^{components/net/mime_classifier.rs, line 205 [r2] (raw file):}
nit: extra space after =

---

^{components/net/mime_classifier.rs, line 209 [r2] (raw file):}
I suspect this could be data[16..box_size].chunks(4).any(|chunk| chunk.starts_with(&mp4)).

---

^{components/net/mime_classifier.rs, line 348 [r2] (raw file):}
Can we make this return a more descriptive enum instead of what is effectively a tri-state bool? Something like DidNotMatch, MatchedStart, MatchedStartAndEnd?

---

^{components/net/mime_classifier.rs, line 350 [r2] (raw file):}
Let's invert this condition and return early instead.

---

^{components/net/mime_classifier.rs, line 352 [r2] (raw file):}
find(...).is_some()

---

^{components/net/mime_classifier.rs, line 353 [r2] (raw file):}
nit: no need for an else after a return

---

^{components/net/mime_classifier.rs, line 357 [r2] (raw file):}
I think I'd prefer:

while !matcher.matches(end) {
    if match.next().is_none() {
        return Some(false);
    }
}

Some(true)

---

^{components/net/mime_classifier.rs, line 383 [r2] (raw file):}
nit:

if ... {
    ...
}

---

^{components/net/mime_classifier.rs, line 394 [r2] (raw file):}
I would prefer

if ... {
   ...
}

rather than all on one line for these.

---

^{components/net/mime_classifier.rs, line 396 [r2] (raw file):}
I would prefer while matcher.next().is_some() {

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 128 [r2] (raw file):}
self and matches ... but we could change that

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 131 [r2] (raw file):}
yep

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 153 [r2] (raw file):}
I think I tried and it didn't at that time. I'll try again tonight or tomorrow night

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 154 [r2] (raw file):}
ok

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 205 [r2] (raw file):}
yes ... we should have it in ./mach test-tidy

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 209 [r2] (raw file):}
nice!
Didn't think of chunk for step_by replacement

---

Comments from the review on Reviewable.io

[tafia]

^{components/net/mime_classifier.rs, line 348 [r2] (raw file):}
sure ...

---

Comments from the review on Reviewable.io

[tafia]

Thanks for the review!
I'll try to address them all as soon as possible.

---

Comments from the review on Reviewable.io

[jdm]

One note for the future - Reviewable allows making multiple comments and publishing all at once :)

[tafia]

Rebased and made all the changes

One note for the future - Reviewable allows making multiple comments and publishing all at once :)

Sorry ... 😄

[jdm]

Go ahead and squash these into a single commit :) I think it's ready to merge, assuming the tests still pass!
-S-awaiting-review -S-needs-rebase

---

Reviewed 1 of 1 files at r3.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from the review on Reviewable.io

[tafia]

Here you go
All unit tests are ok.

[jdm]

@bors-servo: r+
Thanks a lot!

[bors-servo]

📌 Commit dd1c8c8 has been approved by jdm

[tafia]

Thanks for the review!

[bors-servo]

⌛ Testing commit dd1c8c8 with merge 9708c63...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, mac1, mac2, mac3