This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9050

hg-core: add `files_with_nodes` to `Manifest`
ClosedPublic

Authored by acezar on Sep 18 2020, 12:01 PM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGf2de24c2b1f6: hg-core: add `files_with_nodes` to `Manifest`

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

acezar created this revision.Sep 18 2020, 12:01 PM
Alphare accepted this revision.Sep 22 2020, 8:34 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
rust/hg-core/src/revlog/manifest.rs
61

Docstring should mention that it iterates over the files and the hashes.

69

Maybe explain the 40 number and/or reuse from a constant?

70

Possibly this indexing could panic in the case of corrupted data, maybe we should have a more explicit panic message. I'm not certain as doing this everywhere would probably not be worth it. What do you think?

acezar added a commit: rHGf2de24c2b1f6: hg-core: add `files_with_nodes` to `Manifest`.Sep 23 2020, 10:58 AM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHGf2de24c2b1f6: hg-core: add `files_with_nodes` to `Manifest` (authored by acezar). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/revlog/manifest.rs (13 lines)

Diff 22731

rust/hg-core/src/revlog/manifest.rs

self.lines().filter(|line| !line.is_empty()).map(|line| { self.lines().filter(|line| !line.is_empty()).map(|line| {
let pos = line let pos = line
.iter() .iter()
.position(|x| x == &b'\0') .position(|x| x == &b'\0')
.expect("manifest line should contain \\0"); .expect("manifest line should contain \\0");
HgPath::new(&line[..pos]) HgPath::new(&line[..pos])
}) })
} }
/// Return an iterator over the files of the entry.
AlphareUnsubmitted
Not Done

Docstring should mention that it iterates over the files and the hashes.

Alphare: Docstring should mention that it iterates over the files and the hashes.
pub fn files_with_nodes(&self) -> impl Iterator<Item = (&HgPath, &[u8])> {
self.lines().filter(|line| !line.is_empty()).map(|line| {
let pos = line
.iter()
.position(|x| x == &b'\0')
.expect("manifest line should contain \\0");
let hash_start = pos + 1;
let hash_end = hash_start + 40;
AlphareUnsubmitted
Not Done

Maybe explain the 40 number and/or reuse from a constant?

Alphare: Maybe explain the `40` number and/or reuse from a constant?
(HgPath::new(&line[..pos]), &line[hash_start..hash_end])
AlphareUnsubmitted
Not Done

Possibly this indexing could panic in the case of corrupted data, maybe we should have a more explicit panic message. I'm not certain as doing this everywhere would probably not be worth it. What do you think?

Alphare: Possibly this indexing could panic in the case of corrupted data, maybe we should have a more…
})
}
} }