D6568: abort: added support for rebase

2019-07-05 Thread pulkit (Pulkit Goyal) 
pulkit added inline comments.

INLINE COMMENTS

> rebase.py:1926
>  
> +def hgabortrebase(ui, repo, **opts):
> +if opts.get('no_backup'):

we can name this function abortrebase, no need to add hg in front.

> rebase.py:1928
> +if opts.get('no_backup'):
> +raise error.Abort(_("rebase does not support no-backup flag"))
> +if opts.get('dry_run'):

`aborting rebase ..`

> test-rebase-abort.t:128
> +  $ hg abort --dry-run
> +  aborting rebase
> +

This message means that we are aborting rebase. Rather we should print 
something like, `rebase in progress, will be aborted`.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6568/new/

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

To: taapas1128, martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6567: abort: added support for graft

2019-07-05 Thread pulkit (Pulkit Goyal) 
pulkit added a comment.


  > Tests have been shown as test-abort.t
  
  There is no such test now. :)

INLINE COMMENTS

> commands.py:2733
>  
> +def hgabortgraft(ui, repo, **opts):
> +if opts.get('no_backup'):

let's move this function to cmdutil.py. Should move _readgraftstate, 
_abortgraftstate to cmdutil.py too. Movement can be done as a separate patch 
before this patch.

> commands.py:2735
> +if opts.get('no_backup'):
> +raise error.Abort(_("graft does not support no-backup flag"))
> +if opts.get('dry_run'):

`aborting graft ...`

> commands.py:2736
> +raise error.Abort(_("graft does not support no-backup flag"))
> +if opts.get('dry_run'):
> +return 0

We can get rid of this if if we prevent calling abortfunc() in abort command.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6567/new/

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

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6566: abort: added logic for of hg abort

2019-07-05 Thread pulkit (Pulkit Goyal) 
pulkit added inline comments.

INLINE COMMENTS

> commands.py:135
>  
> +@command('abort',
> +[('', 'no-backup', False, _('do not save backup copies of files')),

Can you mark this command as experimental for now, we will be close to release 
soon. Marking this as experimental will help us take more time to smoothen 
things out.

> commands.py:136
> +@command('abort',
> +[('', 'no-backup', False, _('do not save backup copies of files')),
> +] + dryrunopts, helpcategory=command.CATEGORY_CHANGE_MANAGEMENT,

This help should be `'do not save backup bundle'` as similar to strip command.

> commands.py:142
> +
> +Aborts an multistep operation like graft, histedit, rebase, merge,
> +and unshelve if they are in an unfinished state.

s/an/a

> commands.py:154
> +abortstate = None
> +dryrun = opts.get('dry_run')
> +for state in statemod._unfinishedstates:

need to prepend `r''` in front of dry_run for py3 compat.

> commands.py:165
> +if dryrun:
> +ui.status(_('aborting %s\n') % (abortstate._opname))
> +return abortstate.abortfunc(ui, repo, **opts)

we should return here so that `abortstate.abortfunc()` is not called below in 
case of dry-run.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6566/new/

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

To: taapas1128, #hg-reviewers
Cc: pulkit, martinvonz, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6566: abort: added logic for of hg abort

2019-07-05 Thread taapas1128 (Taapas Agrawal) 
taapas1128 updated this revision to Diff 15775.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6566?vs=15750=15775

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6566/new/

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

AFFECTED FILES
  mercurial/commands.py
  mercurial/state.py
  tests/test-blackbox.t
  tests/test-commandserver.t
  tests/test-completion.t
  tests/test-globalopts.t
  tests/test-help-hide.t
  tests/test-help.t
  tests/test-hgweb-json.t

CHANGE DETAILS

diff --git a/tests/test-hgweb-json.t b/tests/test-hgweb-json.t
--- a/tests/test-hgweb-json.t
+++ b/tests/test-hgweb-json.t
@@ -1875,6 +1875,10 @@
   {
 "earlycommands": [
   {
+"summary": "abort an unfinished operation",
+"topic": "abort"
+  },
+  {
 "summary": "add the specified files on the next commit",
 "topic": "add"
   },
diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -5,6 +5,7 @@
   
   basic commands:
   
+   abort abort an unfinished operation
add   add the specified files on the next commit
annotate  show changeset information by line for each file
clone make a copy of an existing repository
@@ -26,6 +27,7 @@
   (use 'hg help' for the full list of commands or 'hg -v' for details)
 
   $ hg -q
+   abort abort an unfinished operation
add   add the specified files on the next commit
annotate  show changeset information by line for each file
clone make a copy of an existing repository
@@ -73,6 +75,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
@@ -201,6 +204,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
@@ -402,6 +406,7 @@
   
   basic commands:
   
+   abort abort an unfinished operation
add   add the specified files on the next commit
annotate, blame
  show changeset information by line for each file
@@ -2353,6 +2358,13 @@
   Main 
Commands
   
   
+  
+  abort
+  
+  
+  abort an unfinished operation
+  
+  
   
   add
   
diff --git a/tests/test-help-hide.t b/tests/test-help-hide.t
--- a/tests/test-help-hide.t
+++ b/tests/test-help-hide.t
@@ -21,6 +21,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
@@ -157,6 +158,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
diff --git a/tests/test-globalopts.t b/tests/test-globalopts.t
--- a/tests/test-globalopts.t
+++ b/tests/test-globalopts.t
@@ -317,6 +317,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
@@ -449,6 +450,7 @@
   
   Change manipulation:
   
+   abort abort an unfinished operation
backout   reverse effect of earlier changeset
graft copy changes from other branches onto the current branch
merge merge another revision into working directory
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -1,5 +1,6 @@
 Show all commands except debug commands
   $ hg debugcomplete
+  abort
   add
   addremove
   annotate
@@ -59,6 +60,7 @@
 
 Show all commands that start with "a"
   $ hg debugcomplete a
+  abort
   add
   addremove
   annotate
@@ -235,6 +237,7 @@
 
 Show all commands + options
   $ hg debugcommands
+  abort: no-backup, dry-run
   add: include, exclude, subrepos, dry-run
   addremove: similarity, subrepos, include, exclude, dry-run
   annotate: rev, follow, no-follow, text, user, file, date, number, changeset, 
line-number, skip, ignore-all-space, ignore-space-change, ignore-blank-lines, 
ignore-space-at-eol, include, exclude, template
diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t
--- a/tests/test-commandserver.t
+++ b/tests/test-commandserver.t
@@ -62,6 +62,7 @@
   
   basic commands:
   
+   abort abort an unfinished

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Raphaël Gomès 
Alphare added a comment.


  In D6593#96417 , @yuja wrote:
  
  >>   Using `keys()` will change the behavior, as I actually need to expose an 
`Iter, u32>`. Is your issue with the naming?
  >
  > No. My point is that the refcount value is an implementation detail.
  > Do you have some plan to use the refcount value?
  >
  >   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
  >   @@ -8,7 +8,7 @@
  >//! A multiset of directory names.
  >//!
  >//! Used to counts the references to directories in a manifest or 
dirstate.
  >   -use std::collections::hash_map::{Entry, Iter};
  >   +use std::collections::hash_map::{Entry, Keys};
  >use std::collections::HashMap;
  >use {DirsIterable, DirstateEntry, DirstateMapError};
  >   @@ -128,8 +128,8 @@ impl DirsMultiset {
  >self.inner.contains_key(key)
  >}
  >   -pub fn iter() -> Iter, u32> {
  >   -self.inner.iter()
  >   +pub fn iter() -> Keys, u32> {
  >   +self.inner.keys()
  >}
  >pub fn len() -> usize {
  >   diff --git a/rust/hg-cpython/src/dirstate.rs 
b/rust/hg-cpython/src/dirstate.rs
  >   --- a/rust/hg-cpython/src/dirstate.rs
  >   +++ b/rust/hg-cpython/src/dirstate.rs
  >   @@ -271,20 +271,9 @@ py_class!(pub class Dirs |py| {
  >// of having it work to continue working on the rest of the module
  >// hopefully bypassing Python entirely pretty soon.
  >def __iter__() -> PyResult {
  >   -let dict = PyDict::new(py);
  >   -
  >   -for (key, value) in self.dirs_map(py).borrow().iter() {
  >   -dict.set_item(
  >   -py,
  >   -PyBytes::new(py, [..]),
  >   -value.to_py_object(py),
  >   -)?;
  >   -}
  >   -
  >   -let locals = PyDict::new(py);
  >   -locals.set_item(py, "obj", dict)?;
  >   -
  >   -py.eval("iter(obj)", None, Some())
  >   +// maybe there would be a better way to cast objects back and 
forth?
  >   +let dirs: Vec<_> = self.dirs_map(py).borrow().iter().map(|d| 
PyBytes::new(py, d)).collect();
  >   +dirs.to_py_object(py).into_object().iter(py).map(|o| 
o.into_object())
  >}
  >def __contains__(, item: PyObject) -> PyResult {
  
  I guess I was trying too much to stick to the exact behavior of the Python 
implementation, but the refcount is indeed not public, that makes sense, thanks.
  
  Regarding your inline comment, my RFC for dirstatemap contains iterator 
sharing between Rust and Python, I would be very interested in what you would 
have to say about it.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread yuja (Yuya Nishihara) 
yuja added a comment.


  >   Using `keys()` will change the behavior, as I actually need to expose an 
`Iter, u32>`. Is your issue with the naming?
  
  No. My point is that the refcount value is an implementation detail.
  Do you have some plan to use the refcount value?
  
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
@@ -8,7 +8,7 @@
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use std::collections::hash_map::{Entry, Iter};
+use std::collections::hash_map::{Entry, Keys};
 use std::collections::HashMap;
 use {DirsIterable, DirstateEntry, DirstateMapError};
 
@@ -128,8 +128,8 @@ impl DirsMultiset {
 self.inner.contains_key(key)
 }
 
-pub fn iter() -> Iter, u32> {
-self.inner.iter()
+pub fn iter() -> Keys, u32> {
+self.inner.keys()
 }
 
 pub fn len() -> usize {
diff --git a/rust/hg-cpython/src/dirstate.rs 
b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -271,20 +271,9 @@ py_class!(pub class Dirs |py| {
 // of having it work to continue working on the rest of the module
 // hopefully bypassing Python entirely pretty soon.
 def __iter__() -> PyResult {
-let dict = PyDict::new(py);
-
-for (key, value) in self.dirs_map(py).borrow().iter() {
-dict.set_item(
-py,
-PyBytes::new(py, [..]),
-value.to_py_object(py),
-)?;
-}
-
-let locals = PyDict::new(py);
-locals.set_item(py, "obj", dict)?;
-
-py.eval("iter(obj)", None, Some())
+// maybe there would be a better way to cast objects back and 
forth?
+let dirs: Vec<_> = self.dirs_map(py).borrow().iter().map(|d| 
PyBytes::new(py, d)).collect();
+dirs.to_py_object(py).into_object().iter(py).map(|o| 
o.into_object())
 }
 
 def __contains__(, item: PyObject) -> PyResult {

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Yuya Nishihara 
>   Using `keys()` will change the behavior, as I actually need to expose an 
> `Iter, u32>`. Is your issue with the naming?

No. My point is that the refcount value is an implementation detail.
Do you have some plan to use the refcount value?

```
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
@@ -8,7 +8,7 @@
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use std::collections::hash_map::{Entry, Iter};
+use std::collections::hash_map::{Entry, Keys};
 use std::collections::HashMap;
 use {DirsIterable, DirstateEntry, DirstateMapError};
 
@@ -128,8 +128,8 @@ impl DirsMultiset {
 self.inner.contains_key(key)
 }
 
-pub fn iter() -> Iter, u32> {
-self.inner.iter()
+pub fn iter() -> Keys, u32> {
+self.inner.keys()
 }
 
 pub fn len() -> usize {
diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -271,20 +271,9 @@ py_class!(pub class Dirs |py| {
 // of having it work to continue working on the rest of the module
 // hopefully bypassing Python entirely pretty soon.
 def __iter__() -> PyResult {
-let dict = PyDict::new(py);
-
-for (key, value) in self.dirs_map(py).borrow().iter() {
-dict.set_item(
-py,
-PyBytes::new(py, [..]),
-value.to_py_object(py),
-)?;
-}
-
-let locals = PyDict::new(py);
-locals.set_item(py, "obj", dict)?;
-
-py.eval("iter(obj)", None, Some())
+// maybe there would be a better way to cast objects back and forth?
+let dirs: Vec<_> = self.dirs_map(py).borrow().iter().map(|d| 
PyBytes::new(py, d)).collect();
+dirs.to_py_object(py).into_object().iter(py).map(|o| o.into_object())
 }
 
 def __contains__(, item: PyObject) -> PyResult {

```
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Raphaël Gomès 
Alphare added a comment.


  
  
  > Again,
  >
  > - **contains**() -> inner.contains_key()
  > - iter() -> inner.**keys**()
  >
  > Somewhat similar to HashSet, which is basically a proxy type to
  > HashMap.
  
  `contains()` is my bad, I forgot.
  
  Using `keys()` will change the behavior, as I actually need to expose an 
`Iter, u32>`. Is your issue with the naming?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6591: rust-minor-fixes: run rfmt on all hg-core/hg-cpython code

2019-07-05 Thread Raphaël Gomès 
Closed by commit rHGd26e4a434fe5: rust: run rfmt on all hg-core/hg-cpython code 
(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/D6591?vs=15727=15772

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6591/new/

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

AFFECTED FILES
  rust/hg-core/src/dirstate/dirs_multiset.rs
  rust/hg-cpython/src/dagops.rs
  rust/hg-cpython/src/exceptions.rs
  rust/hg-cpython/src/lib.rs

CHANGE DETAILS

diff --git a/rust/hg-cpython/src/lib.rs b/rust/hg-cpython/src/lib.rs
--- a/rust/hg-cpython/src/lib.rs
+++ b/rust/hg-cpython/src/lib.rs
@@ -28,9 +28,9 @@
 mod cindex;
 mod conversion;
 pub mod dagops;
+pub mod dirstate;
 pub mod discovery;
 pub mod exceptions;
-pub mod dirstate;
 pub mod filepatterns;
 
 py_module_initializer!(rustext, initrustext, PyInit_rustext, |py, m| {
@@ -45,9 +45,21 @@
 m.add(py, "dagop", dagops::init_module(py, _name)?)?;
 m.add(py, "discovery", discovery::init_module(py, _name)?)?;
 m.add(py, "dirstate", dirstate::init_module(py, _name)?)?;
-m.add(py, "filepatterns", filepatterns::init_module(py, _name)?)?;
+m.add(
+py,
+"filepatterns",
+filepatterns::init_module(py, _name)?,
+)?;
 m.add(py, "GraphError", py.get_type::())?;
-m.add(py, "PatternFileError", 
py.get_type::())?;
-m.add(py, "PatternError", py.get_type::())?;
+m.add(
+py,
+"PatternFileError",
+py.get_type::(),
+)?;
+m.add(
+py,
+"PatternError",
+py.get_type::(),
+)?;
 Ok(())
 });
diff --git a/rust/hg-cpython/src/exceptions.rs 
b/rust/hg-cpython/src/exceptions.rs
--- a/rust/hg-cpython/src/exceptions.rs
+++ b/rust/hg-cpython/src/exceptions.rs
@@ -12,8 +12,8 @@
 //! existing Python exceptions if appropriate.
 //!
 //! [`GraphError`]: struct.GraphError.html
-use cpython::exc::{ValueError, RuntimeError};
-use cpython::{PyErr, Python, exc};
+use cpython::exc::{RuntimeError, ValueError};
+use cpython::{exc, PyErr, Python};
 use hg;
 
 py_exception!(rustext, GraphError, ValueError);
@@ -28,10 +28,10 @@
 match py
 .import("mercurial.error")
 .and_then(|m| m.get(py, "WdirUnsupported"))
-{
-Err(e) => e,
-Ok(cls) => PyErr::from_instance(py, cls),
-}
+{
+Err(e) => e,
+Ok(cls) => PyErr::from_instance(py, cls),
+}
 }
 }
 }
@@ -50,23 +50,18 @@
 }
 }
 
-
 impl PatternFileError {
 pub fn pynew(py: Python, inner: hg::PatternFileError) -> PyErr {
 match inner {
 hg::PatternFileError::IO(e) => {
-let value = (
-e.raw_os_error().unwrap_or(2),
-e.to_string()
-);
+let value = (e.raw_os_error().unwrap_or(2), e.to_string());
 PyErr::new::(py, value)
 }
-hg::PatternFileError::Pattern(e, l) => {
-match e {
-hg::PatternError::UnsupportedSyntax(m) =>
-PatternFileError::new(py, ("PatternFileError", m, l))
+hg::PatternFileError::Pattern(e, l) => match e {
+hg::PatternError::UnsupportedSyntax(m) => {
+PatternFileError::new(py, ("PatternFileError", m, l))
 }
-}
+},
 }
 }
 }
diff --git a/rust/hg-cpython/src/dagops.rs b/rust/hg-cpython/src/dagops.rs
--- a/rust/hg-cpython/src/dagops.rs
+++ b/rust/hg-cpython/src/dagops.rs
@@ -9,9 +9,9 @@
 //! `hg-core` package.
 //!
 //! From Python, this will be seen as `mercurial.rustext.dagop`
+use crate::conversion::{py_set, rev_pyiter_collect};
 use cindex::Index;
 use cpython::{PyDict, PyModule, PyObject, PyResult, Python};
-use crate::conversion::{py_set, rev_pyiter_collect};
 use exceptions::GraphError;
 use hg::dagops;
 use hg::Revision;
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
@@ -118,7 +118,9 @@
 entry.remove();
 }
 Entry::Vacant(_) => {
-return Err(DirstateMapError::PathNotFound(path.to_owned()))
+return Err(DirstateMapError::PathNotFound(
+path.to_owned(),
+))
 }
 };
 



To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Raphaël Gomès 
Closed by commit rHGa80464e85ddd: rust: remove Deref in favor of explicit 
methods (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/D6593?vs=15771=15774

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

AFFECTED FILES
  rust/hg-core/src/dirstate/dirs_multiset.rs

CHANGE DETAILS

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
@@ -8,9 +8,8 @@
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use std::collections::hash_map::Entry;
+use std::collections::hash_map::{Entry, Iter};
 use std::collections::HashMap;
-use std::ops::Deref;
 use {DirsIterable, DirstateEntry, DirstateMapError};
 
 #[derive(PartialEq, Debug)]
@@ -18,14 +17,6 @@
 inner: HashMap, u32>,
 }
 
-impl Deref for DirsMultiset {
-type Target = HashMap, u32>;
-
-fn deref() -> ::Target {
-
-}
-}
-
 impl DirsMultiset {
 /// Initializes the multiset from a dirstate or a manifest.
 ///
@@ -132,6 +123,18 @@
 
 Ok(())
 }
+
+pub fn contains_key(, key: &[u8]) -> bool {
+self.inner.contains_key(key)
+}
+
+pub fn iter() -> Iter, u32> {
+self.inner.iter()
+}
+
+pub fn len() -> usize {
+self.inner.len()
+}
 }
 
 #[cfg(test)]
@@ -176,8 +179,8 @@
 map.delete_path(b"a/b/")
 );
 
-assert_eq!(2, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get("a/c".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/c".to_vec()).unwrap());
 eprintln!("{:?}", map);
 assert_eq!(Ok(()), map.delete_path(b"a/"));
 eprintln!("{:?}", map);
@@ -203,36 +206,36 @@
 let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);
 
 map.add_path(b"a/");
-assert_eq!(1, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get(::new()).unwrap());
+assert_eq!(1, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get(::new()).unwrap());
 assert_eq!(2, map.len());
 
 // Non directory should be ignored
 map.add_path(b"a");
-assert_eq!(1, *map.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a".to_vec()).unwrap());
 assert_eq!(2, map.len());
 
 // Non directory will still add its base
 map.add_path(b"a/b");
-assert_eq!(2, *map.get("a".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a".to_vec()).unwrap());
 assert_eq!(2, map.len());
 
 // Duplicate path works
 map.add_path(b"a/");
-assert_eq!(3, *map.get("a".to_vec()).unwrap());
+assert_eq!(3, *map.inner.get("a".to_vec()).unwrap());
 
 // Nested dir adds to its base
 map.add_path(b"a/b/");
-assert_eq!(4, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get("a/b".to_vec()).unwrap());
+assert_eq!(4, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/b".to_vec()).unwrap());
 
 // but not its base's base, because it already existed
 map.add_path(b"a/b/c/");
-assert_eq!(4, *map.get("a".to_vec()).unwrap());
-assert_eq!(2, *map.get("a/b".to_vec()).unwrap());
+assert_eq!(4, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a/b".to_vec()).unwrap());
 
 map.add_path(b"a/c/");
-assert_eq!(1, *map.get("a/c".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/c".to_vec()).unwrap());
 
 let expected = DirsMultiset {
 inner: [("", 2), ("a", 5), ("a/b", 2), ("a/b/c", 1), ("a/c", 1)]



To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6592: rust-minor-fixes: simplify overly complicated expression

2019-07-05 Thread Raphaël Gomès 
Closed by commit rHG11c025c83393: rust: simplify overly complicated expression 
(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/D6592?vs=15728=15773

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6592/new/

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

AFFECTED FILES
  rust/hg-cpython/src/dirstate.rs

CHANGE DETAILS

diff --git a/rust/hg-cpython/src/dirstate.rs b/rust/hg-cpython/src/dirstate.rs
--- a/rust/hg-cpython/src/dirstate.rs
+++ b/rust/hg-cpython/src/dirstate.rs
@@ -291,8 +291,7 @@
 Ok(self
 .dirs_map(py)
 .borrow()
-.get(::(py)?.data(py).to_owned())
-.is_some())
+.contains_key(item.extract::(py)?.data(py).as_ref()))
 }
 });
 



To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread yuja (Yuya Nishihara) 
yuja added a comment.


  > +pub fn contains_key(, key: &[u8]) -> bool {
  > +self.inner.contains_key(key)
  > +}
  > +
  > +pub fn iter() -> Iter, u32> {
  > +self.inner.iter()
  > +}
  
  Again,
  
  - **contains**() -> inner.contains_key()
  - iter() -> inner.**keys**()
  
  Somewhat similar to HashSet, which is basically a proxy type to
  HashMap.
  
  I queued this as I think it's strictly better than using Deref, but I expect
  a follow up patch. Thanks.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Yuya Nishihara 
> +pub fn contains_key(, key: &[u8]) -> bool {
> +self.inner.contains_key(key)
> +}
> +
> +pub fn iter() -> Iter, u32> {
> +self.inner.iter()
> +}

Again,

- **contains**() -> inner.contains_key()
- iter() -> inner.**keys**()

Somewhat similar to HashSet, which is basically a proxy type to
HashMap.

I queued this as I think it's strictly better than using Deref, but I expect
a follow up patch. Thanks.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

2019-07-05 Thread Raphaël Gomès 
Alphare updated this revision to Diff 15771.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D6593?vs=15740=15771

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6593/new/

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

AFFECTED FILES
  rust/hg-core/src/dirstate/dirs_multiset.rs

CHANGE DETAILS

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
@@ -8,9 +8,8 @@
 //! A multiset of directory names.
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
-use std::collections::hash_map::Entry;
+use std::collections::hash_map::{Entry, Iter};
 use std::collections::HashMap;
-use std::ops::Deref;
 use {DirsIterable, DirstateEntry, DirstateMapError};
 
 #[derive(PartialEq, Debug)]
@@ -18,14 +17,6 @@
 inner: HashMap, u32>,
 }
 
-impl Deref for DirsMultiset {
-type Target = HashMap, u32>;
-
-fn deref() -> ::Target {
-
-}
-}
-
 impl DirsMultiset {
 /// Initializes the multiset from a dirstate or a manifest.
 ///
@@ -132,6 +123,18 @@
 
 Ok(())
 }
+
+pub fn contains_key(, key: &[u8]) -> bool {
+self.inner.contains_key(key)
+}
+
+pub fn iter() -> Iter, u32> {
+self.inner.iter()
+}
+
+pub fn len() -> usize {
+self.inner.len()
+}
 }
 
 #[cfg(test)]
@@ -176,8 +179,8 @@
 map.delete_path(b"a/b/")
 );
 
-assert_eq!(2, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get("a/c".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/c".to_vec()).unwrap());
 eprintln!("{:?}", map);
 assert_eq!(Ok(()), map.delete_path(b"a/"));
 eprintln!("{:?}", map);
@@ -203,36 +206,36 @@
 let mut map = DirsMultiset::new(DirsIterable::Manifest(vec![]), None);
 
 map.add_path(b"a/");
-assert_eq!(1, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get(::new()).unwrap());
+assert_eq!(1, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get(::new()).unwrap());
 assert_eq!(2, map.len());
 
 // Non directory should be ignored
 map.add_path(b"a");
-assert_eq!(1, *map.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a".to_vec()).unwrap());
 assert_eq!(2, map.len());
 
 // Non directory will still add its base
 map.add_path(b"a/b");
-assert_eq!(2, *map.get("a".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a".to_vec()).unwrap());
 assert_eq!(2, map.len());
 
 // Duplicate path works
 map.add_path(b"a/");
-assert_eq!(3, *map.get("a".to_vec()).unwrap());
+assert_eq!(3, *map.inner.get("a".to_vec()).unwrap());
 
 // Nested dir adds to its base
 map.add_path(b"a/b/");
-assert_eq!(4, *map.get("a".to_vec()).unwrap());
-assert_eq!(1, *map.get("a/b".to_vec()).unwrap());
+assert_eq!(4, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/b".to_vec()).unwrap());
 
 // but not its base's base, because it already existed
 map.add_path(b"a/b/c/");
-assert_eq!(4, *map.get("a".to_vec()).unwrap());
-assert_eq!(2, *map.get("a/b".to_vec()).unwrap());
+assert_eq!(4, *map.inner.get("a".to_vec()).unwrap());
+assert_eq!(2, *map.inner.get("a/b".to_vec()).unwrap());
 
 map.add_path(b"a/c/");
-assert_eq!(1, *map.get("a/c".to_vec()).unwrap());
+assert_eq!(1, *map.inner.get("a/c".to_vec()).unwrap());
 
 let expected = DirsMultiset {
 inner: [("", 2), ("a", 5), ("a/b", 2), ("a/b/c", 1), ("a/c", 1)]



To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel