[Request] [+-- ] D11383: pathutil: replace the `skip` argument of `dirs` with a boolean

marmoute (Pierre-Yves David) phabricator at mercurial-scm.org
Wed Sep 1 23:36:43 UTC 2021


marmoute created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  It is ever only used for `r` file. So we make it a boolean this will give use
  more versatility later as we will stop storing the state explicitly.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D11383

AFFECTED FILES
  mercurial/cext/dirs.c
  mercurial/dirstatemap.py
  mercurial/pathutil.py
  rust/hg-core/src/dirstate/dirs_multiset.rs
  rust/hg-core/src/dirstate/dirstate_map.rs
  rust/hg-cpython/src/dirstate/dirs_multiset.rs

CHANGE DETAILS

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
@@ -9,19 +9,16 @@
 //! `hg-core` package.
 
 use std::cell::RefCell;
-use std::convert::TryInto;
 
 use cpython::{
-    exc, ObjectProtocol, PyBytes, PyClone, PyDict, PyErr, PyObject, PyResult,
-    Python, UnsafePyLeaked,
+    exc, ObjectProtocol, PyBool, PyBytes, PyClone, PyDict, PyErr, PyObject,
+    PyResult, Python, UnsafePyLeaked,
 };
 
 use crate::dirstate::extract_dirstate;
 use hg::{
-    errors::HgError,
     utils::hg_path::{HgPath, HgPathBuf},
     DirsMultiset, DirsMultisetIter, DirstateError, DirstateMapError,
-    EntryState,
 };
 
 py_class!(pub class Dirs |py| {
@@ -32,22 +29,17 @@
     def __new__(
         _cls,
         map: PyObject,
-        skip: Option<PyObject> = None
+        only_tracked: Option<PyObject> = None
     ) -> PyResult<Self> {
-        let mut skip_state: Option<EntryState> = None;
-        if let Some(skip) = skip {
-            skip_state = Some(
-                skip.extract::<PyBytes>(py)?.data(py)[0]
-                    .try_into()
-                    .map_err(|e: HgError| {
-                        PyErr::new::<exc::ValueError, _>(py, e.to_string())
-                    })?,
-            );
-        }
+        let only_tracked_b = if let Some(only_tracked) = only_tracked {
+            only_tracked.extract::<PyBool>(py)?.is_true()
+        } else {
+            false
+        };
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
             let dirstate = dirstate.iter().map(|(k, v)| Ok((k, *v)));
-            DirsMultiset::from_dirstate(dirstate, skip_state)
+            DirsMultiset::from_dirstate(dirstate, only_tracked_b)
                 .map_err(|e: DirstateError| {
                     PyErr::new::<exc::ValueError, _>(py, e.to_string())
                 })?
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
@@ -334,7 +334,7 @@
         if self.all_dirs.is_none() {
             self.all_dirs = Some(DirsMultiset::from_dirstate(
                 self.state_map.iter().map(|(k, v)| Ok((k, *v))),
-                None,
+                false,
             )?);
         }
         Ok(())
@@ -344,7 +344,7 @@
         if self.dirs.is_none() {
             self.dirs = Some(DirsMultiset::from_dirstate(
                 self.state_map.iter().map(|(k, v)| Ok((k, *v))),
-                Some(EntryState::Removed),
+                true,
             )?);
         }
         Ok(())
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
@@ -33,7 +33,7 @@
     /// If `skip_state` is provided, skips dirstate entries with equal state.
     pub fn from_dirstate<I, P>(
         dirstate: I,
-        skip_state: Option<EntryState>,
+        only_tracked: bool,
     ) -> Result<Self, DirstateError>
     where
         I: IntoIterator<
@@ -48,8 +48,8 @@
             let (filename, entry) = item?;
             let filename = filename.as_ref();
             // This `if` is optimized out of the loop
-            if let Some(skip) = skip_state {
-                if skip != entry.state {
+            if only_tracked {
+                if entry.state != EntryState::Removed {
                     multiset.add_path(filename)?;
                 }
             } else {
@@ -343,7 +343,7 @@
 
         let new = DirsMultiset::from_dirstate(
             StateMap::default().into_iter().map(Ok),
-            None,
+            false,
         )
         .unwrap();
         let expected = DirsMultiset {
@@ -385,7 +385,7 @@
             .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
             .collect();
 
-        let new = DirsMultiset::from_dirstate(input_map, None).unwrap();
+        let new = DirsMultiset::from_dirstate(input_map, false).unwrap();
         let expected = DirsMultiset {
             inner: expected_inner,
         };
@@ -414,14 +414,12 @@
         });
 
         // "a" incremented with "a/c" and "a/d/"
-        let expected_inner = [("", 1), ("a", 2)]
+        let expected_inner = [("", 1), ("a", 3)]
             .iter()
             .map(|(k, v)| (HgPathBuf::from_bytes(k.as_bytes()), *v))
             .collect();
 
-        let new =
-            DirsMultiset::from_dirstate(input_map, Some(EntryState::Normal))
-                .unwrap();
+        let new = DirsMultiset::from_dirstate(input_map, true).unwrap();
         let expected = DirsMultiset {
             inner: expected_inner,
         };
diff --git a/mercurial/pathutil.py b/mercurial/pathutil.py
--- a/mercurial/pathutil.py
+++ b/mercurial/pathutil.py
@@ -315,20 +315,19 @@
 class dirs(object):
     '''a multiset of directory names from a set of file paths'''
 
-    def __init__(self, map, skip=None):
+    def __init__(self, map, only_tracked=False):
         """
         a dict map indicates a dirstate while a list indicates a manifest
         """
         self._dirs = {}
         addpath = self.addpath
-        if isinstance(map, dict) and skip is not None:
+        if isinstance(map, dict) and only_tracked:
             for f, s in pycompat.iteritems(map):
-                if s.state != skip:
+                if s.state != b'r':
                     addpath(f)
-        elif skip is not None:
-            raise error.ProgrammingError(
-                b"skip character is only supported with a dict source"
-            )
+        elif only_tracked:
+            msg = b"`only_tracked` is only supported with a dict source"
+            raise error.ProgrammingError(msg)
         else:
             for f in map:
                 addpath(f)
diff --git a/mercurial/dirstatemap.py b/mercurial/dirstatemap.py
--- a/mercurial/dirstatemap.py
+++ b/mercurial/dirstatemap.py
@@ -377,7 +377,7 @@
 
     @propertycache
     def _dirs(self):
-        return pathutil.dirs(self._map, b'r')
+        return pathutil.dirs(self._map, only_tracked=True)
 
     @propertycache
     def _alldirs(self):
diff --git a/mercurial/cext/dirs.c b/mercurial/cext/dirs.c
--- a/mercurial/cext/dirs.c
+++ b/mercurial/cext/dirs.c
@@ -161,7 +161,7 @@
 	return ret;
 }
 
-static int dirs_fromdict(PyObject *dirs, PyObject *source, char skipchar)
+static int dirs_fromdict(PyObject *dirs, PyObject *source, bool only_tracked)
 {
 	PyObject *key, *value;
 	Py_ssize_t pos = 0;
@@ -171,13 +171,13 @@
 			PyErr_SetString(PyExc_TypeError, "expected string key");
 			return -1;
 		}
-		if (skipchar) {
+		if (only_tracked) {
 			if (!dirstate_tuple_check(value)) {
 				PyErr_SetString(PyExc_TypeError,
 				                "expected a dirstate tuple");
 				return -1;
 			}
-			if (((dirstateItemObject *)value)->state == skipchar)
+			if (((dirstateItemObject *)value)->state == 'r')
 				continue;
 		}
 
@@ -218,15 +218,17 @@
  * Calculate a refcounted set of directory names for the files in a
  * dirstate.
  */
-static int dirs_init(dirsObject *self, PyObject *args)
+static int dirs_init(dirsObject *self, PyObject *args, PyObject *kwargs)
 {
 	PyObject *dirs = NULL, *source = NULL;
-	char skipchar = 0;
+	int only_tracked = 0;
 	int ret = -1;
+	static char *keywords_name[] = {"map", "only_tracked", NULL};
 
 	self->dict = NULL;
 
-	if (!PyArg_ParseTuple(args, "|Oc:__init__", &source, &skipchar))
+	if (!PyArg_ParseTupleAndKeywords(args, kwargs, "|Oi:__init__",
+	                                 keywords_name, &source, &only_tracked))
 		return -1;
 
 	dirs = PyDict_New();
@@ -237,10 +239,10 @@
 	if (source == NULL)
 		ret = 0;
 	else if (PyDict_Check(source))
-		ret = dirs_fromdict(dirs, source, skipchar);
-	else if (skipchar)
+		ret = dirs_fromdict(dirs, source, (bool)only_tracked);
+	else if (only_tracked)
 		PyErr_SetString(PyExc_ValueError,
-		                "skip character is only supported "
+		                "`only_tracked` is only supported "
 		                "with a dict source");
 	else
 		ret = dirs_fromiter(dirs, source);



To: marmoute, #hg-reviewers
Cc: mercurial-patches, mercurial-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mercurial-scm.org/pipermail/mercurial-patches/attachments/20210901/738e0c84/attachment-0001.html>


More information about the Mercurial-patches mailing list