[Request] [+-- ] D9864: rust: Remove hex parsing from the nodemap
SimonSapin
phabricator at mercurial-scm.org
Mon Jan 25 21:14:02 UTC 2021
SimonSapin created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.
REVISION SUMMARY
Separating concerns simplifies error types.
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D9864
AFFECTED FILES
rust/hg-core/examples/nodemap/main.rs
rust/hg-core/src/revlog/nodemap.rs
rust/hg-cpython/src/revlog.rs
CHANGE DETAILS
diff --git a/rust/hg-cpython/src/revlog.rs b/rust/hg-cpython/src/revlog.rs
--- a/rust/hg-cpython/src/revlog.rs
+++ b/rust/hg-cpython/src/revlog.rs
@@ -17,7 +17,7 @@
};
use hg::{
nodemap::{Block, NodeMapError, NodeTree},
- revlog::{nodemap::NodeMap, RevlogIndex},
+ revlog::{nodemap::NodeMap, NodePrefix, RevlogIndex},
Revision,
};
use std::cell::RefCell;
@@ -107,7 +107,9 @@
String::from_utf8_lossy(node.data(py)).to_string()
};
- nt.find_hex(idx, &node_as_string)
+ let prefix = NodePrefix::from_hex(&node_as_string).map_err(|_| PyErr::new::<ValueError, _>(py, "Invalid node or prefix"))?;
+
+ nt.find_bin(idx, prefix)
// TODO make an inner API returning the node directly
.map(|opt| opt.map(
|rev| PyBytes::new(py, idx.node(rev).unwrap().as_bytes())))
@@ -468,9 +470,6 @@
match err {
NodeMapError::MultipleResults => revlog_error(py),
NodeMapError::RevisionNotInIndex(r) => rev_not_in_index(py, r),
- NodeMapError::InvalidNodePrefix => {
- PyErr::new::<ValueError, _>(py, "Invalid node or prefix")
- }
}
}
diff --git a/rust/hg-core/src/revlog/nodemap.rs b/rust/hg-core/src/revlog/nodemap.rs
--- a/rust/hg-core/src/revlog/nodemap.rs
+++ b/rust/hg-core/src/revlog/nodemap.rs
@@ -13,8 +13,7 @@
//! is used in a more abstract context.
use super::{
- node::NULL_NODE, FromHexError, Node, NodePrefix, Revision, RevlogIndex,
- NULL_REVISION,
+ node::NULL_NODE, Node, NodePrefix, Revision, RevlogIndex, NULL_REVISION,
};
use std::cmp::max;
@@ -27,17 +26,10 @@
#[derive(Debug, PartialEq)]
pub enum NodeMapError {
MultipleResults,
- InvalidNodePrefix,
/// A `Revision` stored in the nodemap could not be found in the index
RevisionNotInIndex(Revision),
}
-impl From<FromHexError> for NodeMapError {
- fn from(_: FromHexError) -> Self {
- NodeMapError::InvalidNodePrefix
- }
-}
-
/// Mapping system from Mercurial nodes to revision numbers.
///
/// ## `RevlogIndex` and `NodeMap`
@@ -85,21 +77,6 @@
prefix: NodePrefix,
) -> Result<Option<Revision>, NodeMapError>;
- /// Find the unique Revision whose `Node` hexadecimal string representation
- /// starts with a given prefix
- ///
- /// If no Revision matches the given prefix, `Ok(None)` is returned.
- ///
- /// If several Revisions match the given prefix, a [`MultipleResults`]
- /// error is returned.
- fn find_hex(
- &self,
- idx: &impl RevlogIndex,
- prefix: &str,
- ) -> Result<Option<Revision>, NodeMapError> {
- self.find_bin(idx, NodePrefix::from_hex(prefix)?)
- }
-
/// Give the size of the shortest node prefix that determines
/// the revision uniquely.
///
@@ -117,16 +94,6 @@
node_prefix: NodePrefix,
) -> Result<Option<usize>, NodeMapError>;
- /// Same as `unique_prefix_len_bin`, with the hexadecimal representation
- /// of the prefix as input.
- fn unique_prefix_len_hex(
- &self,
- idx: &impl RevlogIndex,
- prefix: &str,
- ) -> Result<Option<usize>, NodeMapError> {
- self.unique_prefix_len_bin(idx, NodePrefix::from_hex(prefix)?)
- }
-
/// Same as `unique_prefix_len_bin`, with a full `Node` as input
fn unique_prefix_len_node(
&self,
@@ -819,6 +786,10 @@
])
}
+ fn hex(s: &str) -> NodePrefix {
+ NodePrefix::from_hex(s).unwrap()
+ }
+
#[test]
fn test_nt_debug() {
let nt = sample_nodetree();
@@ -837,11 +808,11 @@
pad_insert(&mut idx, 1, "1234deadcafe");
let nt = NodeTree::from(vec![block! {1: Rev(1)}]);
- assert_eq!(nt.find_hex(&idx, "1")?, Some(1));
- assert_eq!(nt.find_hex(&idx, "12")?, Some(1));
- assert_eq!(nt.find_hex(&idx, "1234de")?, Some(1));
- assert_eq!(nt.find_hex(&idx, "1a")?, None);
- assert_eq!(nt.find_hex(&idx, "ab")?, None);
+ assert_eq!(nt.find_bin(&idx, hex("1"))?, Some(1));
+ assert_eq!(nt.find_bin(&idx, hex("12"))?, Some(1));
+ assert_eq!(nt.find_bin(&idx, hex("1234de"))?, Some(1));
+ assert_eq!(nt.find_bin(&idx, hex("1a"))?, None);
+ assert_eq!(nt.find_bin(&idx, hex("ab"))?, None);
// and with full binary Nodes
assert_eq!(nt.find_node(&idx, idx.get(&1).unwrap())?, Some(1));
@@ -858,12 +829,12 @@
let nt = sample_nodetree();
- assert_eq!(nt.find_hex(&idx, "0"), Err(MultipleResults));
- assert_eq!(nt.find_hex(&idx, "01"), Ok(Some(9)));
- assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
- assert_eq!(nt.find_hex(&idx, "00a"), Ok(Some(0)));
- assert_eq!(nt.unique_prefix_len_hex(&idx, "00a"), Ok(Some(3)));
- assert_eq!(nt.find_hex(&idx, "000"), Ok(Some(NULL_REVISION)));
+ assert_eq!(nt.find_bin(&idx, hex("0")), Err(MultipleResults));
+ assert_eq!(nt.find_bin(&idx, hex("01")), Ok(Some(9)));
+ assert_eq!(nt.find_bin(&idx, hex("00")), Err(MultipleResults));
+ assert_eq!(nt.find_bin(&idx, hex("00a")), Ok(Some(0)));
+ assert_eq!(nt.unique_prefix_len_bin(&idx, hex("00a")), Ok(Some(3)));
+ assert_eq!(nt.find_bin(&idx, hex("000")), Ok(Some(NULL_REVISION)));
}
#[test]
@@ -881,13 +852,13 @@
root: block![0: Block(1), 1:Block(3), 12: Rev(2)],
masked_inner_blocks: 1,
};
- assert_eq!(nt.find_hex(&idx, "10")?, Some(1));
- assert_eq!(nt.find_hex(&idx, "c")?, Some(2));
- assert_eq!(nt.unique_prefix_len_hex(&idx, "c")?, Some(1));
- assert_eq!(nt.find_hex(&idx, "00"), Err(MultipleResults));
- assert_eq!(nt.find_hex(&idx, "000")?, Some(NULL_REVISION));
- assert_eq!(nt.unique_prefix_len_hex(&idx, "000")?, Some(3));
- assert_eq!(nt.find_hex(&idx, "01")?, Some(9));
+ assert_eq!(nt.find_bin(&idx, hex("10"))?, Some(1));
+ assert_eq!(nt.find_bin(&idx, hex("c"))?, Some(2));
+ assert_eq!(nt.unique_prefix_len_bin(&idx, hex("c"))?, Some(1));
+ assert_eq!(nt.find_bin(&idx, hex("00")), Err(MultipleResults));
+ assert_eq!(nt.find_bin(&idx, hex("000"))?, Some(NULL_REVISION));
+ assert_eq!(nt.unique_prefix_len_bin(&idx, hex("000"))?, Some(3));
+ assert_eq!(nt.find_bin(&idx, hex("01"))?, Some(9));
assert_eq!(nt.masked_readonly_blocks(), 2);
Ok(())
}
@@ -920,14 +891,14 @@
&self,
prefix: &str,
) -> Result<Option<Revision>, NodeMapError> {
- self.nt.find_hex(&self.index, prefix)
+ self.nt.find_bin(&self.index, hex(prefix))
}
fn unique_prefix_len_hex(
&self,
prefix: &str,
) -> Result<Option<usize>, NodeMapError> {
- self.nt.unique_prefix_len_hex(&self.index, prefix)
+ self.nt.unique_prefix_len_bin(&self.index, hex(prefix))
}
/// Drain `added` and restart a new one
diff --git a/rust/hg-core/examples/nodemap/main.rs b/rust/hg-core/examples/nodemap/main.rs
--- a/rust/hg-core/examples/nodemap/main.rs
+++ b/rust/hg-core/examples/nodemap/main.rs
@@ -49,7 +49,7 @@
fn query(index: &Index, nm: &NodeTree, prefix: &str) {
let start = Instant::now();
- let res = nm.find_hex(index, prefix);
+ let res = NodePrefix::from_hex(prefix).map(|p| nm.find_bin(index, p));
println!("Result found in {:?}: {:?}", start.elapsed(), res);
}
To: SimonSapin, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20210125/90ca7946/attachment-0001.html>
More information about the Mercurial-patches
mailing list