D7609: rust-dirs: handle forgotten `Result`s
Alphare (Raphaël Gomès)
phabricator at mercurial-scm.org
Fri Dec 13 12:59:06 EST 2019
Closed by commit rHGbc7d8f45c3b6: rust-dirs: handle forgotten `Result`s (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D7609?vs=18686&id=18687
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7609/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7609
AFFECTED FILES
rust/hg-core/src/dirstate/dirs_multiset.rs
rust/hg-core/src/dirstate/dirstate_map.rs
rust/hg-core/src/matchers.rs
rust/hg-cpython/src/dirstate/dirs_multiset.rs
rust/hg-cpython/src/dirstate/dirstate_map.rs
CHANGE DETAILS
diff --git a/rust/hg-cpython/src/dirstate/dirstate_map.rs b/rust/hg-cpython/src/dirstate/dirstate_map.rs
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs
@@ -200,6 +200,9 @@
let d = d.extract::<PyBytes>(py)?;
Ok(self.inner_shared(py).borrow_mut()?
.has_tracked_dir(HgPath::new(d.data(py)))
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
.to_py_object(py))
}
@@ -207,6 +210,9 @@
let d = d.extract::<PyBytes>(py)?;
Ok(self.inner_shared(py).borrow_mut()?
.has_dir(HgPath::new(d.data(py)))
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
.to_py_object(py))
}
@@ -330,24 +336,35 @@
def getdirs(&self) -> PyResult<Dirs> {
// TODO don't copy, share the reference
- self.inner_shared(py).borrow_mut()?.set_dirs();
+ self.inner_shared(py).borrow_mut()?.set_dirs()
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?;
Dirs::from_inner(
py,
DirsMultiset::from_dirstate(
&self.inner_shared(py).borrow(),
Some(EntryState::Removed),
- ),
+ )
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?,
)
}
def getalldirs(&self) -> PyResult<Dirs> {
// TODO don't copy, share the reference
- self.inner_shared(py).borrow_mut()?.set_all_dirs();
+ self.inner_shared(py).borrow_mut()?.set_all_dirs()
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?;
Dirs::from_inner(
py,
DirsMultiset::from_dirstate(
&self.inner_shared(py).borrow(),
None,
- ),
+ ).map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?,
)
}
diff --git a/rust/hg-cpython/src/dirstate/dirs_multiset.rs b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs
@@ -47,6 +47,9 @@
let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
let dirstate = extract_dirstate(py, &map)?;
DirsMultiset::from_dirstate(&dirstate, skip_state)
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
} else {
let map: Result<Vec<HgPathBuf>, PyErr> = map
.iter(py)?
@@ -57,6 +60,9 @@
})
.collect();
DirsMultiset::from_manifest(&map?)
+ .map_err(|e| {
+ PyErr::new::<exc::ValueError, _>(py, e.to_string())
+ })?
};
Self::create_instance(
diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
--- a/rust/hg-core/src/matchers.rs
+++ b/rust/hg-core/src/matchers.rs
@@ -7,7 +7,7 @@
//! Structs and types for matching files and directories.
-use crate::utils::hg_path::{HgPath, HgPathBuf};
+use crate::utils::hg_path::HgPath;
use std::collections::HashSet;
pub enum VisitChildrenSet<'a> {
diff --git a/rust/hg-core/src/dirstate/dirstate_map.rs b/rust/hg-core/src/dirstate/dirstate_map.rs
--- a/rust/hg-core/src/dirstate/dirstate_map.rs
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs
@@ -126,7 +126,7 @@
}
if old_state == EntryState::Unknown {
if let Some(ref mut all_dirs) = self.all_dirs {
- all_dirs.add_path(filename);
+ all_dirs.add_path(filename)?;
}
}
@@ -227,30 +227,38 @@
/// emulate a Python lazy property, but it is ugly and unidiomatic.
/// TODO One day, rewriting this struct using the typestate might be a
/// good idea.
- pub fn set_all_dirs(&mut self) {
+ pub fn set_all_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.all_dirs.is_none() {
self.all_dirs =
- Some(DirsMultiset::from_dirstate(&self.state_map, None));
+ Some(DirsMultiset::from_dirstate(&self.state_map, None)?);
}
+ Ok(())
}
- pub fn set_dirs(&mut self) {
+ pub fn set_dirs(&mut self) -> Result<(), DirstateMapError> {
if self.dirs.is_none() {
self.dirs = Some(DirsMultiset::from_dirstate(
&self.state_map,
Some(EntryState::Removed),
- ));
+ )?);
}
+ Ok(())
}
- pub fn has_tracked_dir(&mut self, directory: &HgPath) -> bool {
- self.set_dirs();
- self.dirs.as_ref().unwrap().contains(directory)
+ pub fn has_tracked_dir(
+ &mut self,
+ directory: &HgPath,
+ ) -> Result<bool, DirstateMapError> {
+ self.set_dirs()?;
+ Ok(self.dirs.as_ref().unwrap().contains(directory))
}
- pub fn has_dir(&mut self, directory: &HgPath) -> bool {
- self.set_all_dirs();
- self.all_dirs.as_ref().unwrap().contains(directory)
+ pub fn has_dir(
+ &mut self,
+ directory: &HgPath,
+ ) -> Result<bool, DirstateMapError> {
+ self.set_all_dirs()?;
+ Ok(self.all_dirs.as_ref().unwrap().contains(directory))
}
pub fn parents(
@@ -350,11 +358,11 @@
assert!(map.dirs.is_none());
assert!(map.all_dirs.is_none());
- assert_eq!(false, map.has_dir(HgPath::new(b"nope")));
+ assert_eq!(map.has_dir(HgPath::new(b"nope")).unwrap(), false);
assert!(map.all_dirs.is_some());
assert!(map.dirs.is_none());
- assert_eq!(false, map.has_tracked_dir(HgPath::new(b"nope")));
+ assert_eq!(map.has_tracked_dir(HgPath::new(b"nope")).unwrap(), false);
assert!(map.dirs.is_some());
}
diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs
--- a/rust/hg-core/src/dirstate/dirs_multiset.rs
+++ b/rust/hg-core/src/dirstate/dirs_multiset.rs
@@ -30,7 +30,7 @@
pub fn from_dirstate(
dirstate: &FastHashMap<HgPathBuf, DirstateEntry>,
skip_state: Option<EntryState>,
- ) -> Self {
+ ) -> Result<Self, DirstateMapError> {
let mut multiset = DirsMultiset {
inner: FastHashMap::default(),
};
@@ -39,27 +39,29 @@
// This `if` is optimized out of the loop
if let Some(skip) = skip_state {
if skip != *state {
- multiset.add_path(filename);
+ multiset.add_path(filename)?;
}
} else {
- multiset.add_path(filename);
+ multiset.add_path(filename)?;
}
}
- multiset
+ Ok(multiset)
}
/// Initializes the multiset from a manifest.
- pub fn from_manifest(manifest: &[impl AsRef<HgPath>]) -> Self {
+ pub fn from_manifest(
+ manifest: &[impl AsRef<HgPath>],
+ ) -> Result<Self, DirstateMapError> {
let mut multiset = DirsMultiset {
inner: FastHashMap::default(),
};
for filename in manifest {
- multiset.add_path(filename.as_ref());
+ multiset.add_path(filename.as_ref())?;
}
- multiset
+ Ok(multiset)
}
/// Increases the count of deepest directory contained in the path.
@@ -134,7 +136,7 @@
#[test]
fn test_delete_path_path_not_found() {
let manifest: Vec<HgPathBuf> = vec![];
- let mut map = DirsMultiset::from_manifest(&manifest);
+ let mut map = DirsMultiset::from_manifest(&manifest).unwrap();
let path = HgPathBuf::from_bytes(b"doesnotexist/");
assert_eq!(
Err(DirstateMapError::PathNotFound(path.to_owned())),
@@ -144,7 +146,8 @@
#[test]
fn test_delete_path_empty_path() {
- let mut map = DirsMultiset::from_manifest(&vec![HgPathBuf::new()]);
+ let mut map =
+ DirsMultiset::from_manifest(&vec![HgPathBuf::new()]).unwrap();
let path = HgPath::new(b"");
assert_eq!(Ok(()), map.delete_path(path));
assert_eq!(
@@ -191,7 +194,7 @@
#[test]
fn test_add_path_empty_path() {
let manifest: Vec<HgPathBuf> = vec![];
- let mut map = DirsMultiset::from_manifest(&manifest);
+ let mut map = DirsMultiset::from_manifest(&manifest).unwrap();
let path = HgPath::new(b"");
map.add_path(path);
@@ -201,7 +204,7 @@
#[test]
fn test_add_path_successful() {
let manifest: Vec<HgPathBuf> = vec![];
- let mut map = DirsMultiset::from_manifest(&manifest);
+ let mut map = DirsMultiset::from_manifest(&manifest).unwrap();
map.add_path(HgPath::new(b"a/"));
assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap());
@@ -247,13 +250,14 @@
#[test]
fn test_dirsmultiset_new_empty() {
let manifest: Vec<HgPathBuf> = vec![];
- let new = DirsMultiset::from_manifest(&manifest);
+ let new = DirsMultiset::from_manifest(&manifest).unwrap();
let expected = DirsMultiset {
inner: FastHashMap::default(),
};
assert_eq!(expected, new);
- let new = DirsMultiset::from_dirstate(&FastHashMap::default(), None);
+ let new = DirsMultiset::from_dirstate(&FastHashMap::default(), None)
+ .unwrap();
let expected = DirsMultiset {
inner: FastHashMap::default(),
};
@@ -271,7 +275,7 @@
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect();
- let new = DirsMultiset::from_manifest(&input_vec);
+ let new = DirsMultiset::from_manifest(&input_vec).unwrap();
let expected = DirsMultiset {
inner: expected_inner,
};
@@ -296,7 +300,7 @@
.map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
.collect();
- let new = DirsMultiset::from_dirstate(&input_map, None);
+ let new = DirsMultiset::from_dirstate(&input_map, None).unwrap();
let expected = DirsMultiset {
inner: expected_inner,
};
@@ -332,7 +336,8 @@
.collect();
let new =
- DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal));
+ DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal))
+ .unwrap();
let expected = DirsMultiset {
inner: expected_inner,
};
To: Alphare, #hg-reviewers
Cc: martinvonz, durin42, kevincox, mercurial-devel
More information about the Mercurial-devel
mailing list