Patchwork D12155: sparse: rework debugsparse's interface

login
register
mail settings
Submitter phabricator
Date Feb. 9, 2022, 4:33 a.m.
Message ID <differential-rev-PHID-DREV-lqpltjplyyxnefqmxg3t-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/50490/
State New
Headers show

Comments

phabricator - Feb. 9, 2022, 4:33 a.m.
valentin.gatienbaron created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  hg debugsparse supports arguments like --include, similar to `hg
  tracked --addinclude` or `hg log --include`. But in `hg debugsparse`,
  the pattern is not an argument of the flag, instead the patterns are
  the anonymous command line arguments.
  
  Not only is this surprising, it makes it impossible to use --include
  and --exclude in the same invocation, or --reset --exclude.
  
  So I propose making debugsparse making --include, --exclude take an
  argument, and rejecting anonymous command line arguments, as well as
  allowing mixing several of these flags in one invocations.
  
  As an implementation remark, I used the:
  
  def f(a, *, b=None):
  
  syntax to require the keyword of keyword arguments. That won't work
  with py2. I don't know if we have dropped py2 by now or not.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  hgext/sparse.py
  mercurial/sparse.py
  tests/test-sparse-clear.t
  tests/test-sparse.t

CHANGE DETAILS




To: valentin.gatienbaron, #hg-reviewers
Cc: mercurial-patches, mercurial-devel

Patch

diff --git a/tests/test-sparse.t b/tests/test-sparse.t
--- a/tests/test-sparse.t
+++ b/tests/test-sparse.t
@@ -147,7 +147,7 @@ 
 
 Verify deleting sparseness with --force brings back files
 
-  $ hg debugsparse --delete -f 'show*'
+  $ hg debugsparse -f --delete 'show*'
   pending changes to 'hide'
   $ ls -A
   .hg
@@ -170,7 +170,7 @@ 
 
 Verify adding sparseness hides files
 
-  $ hg debugsparse --exclude -f 'hide*'
+  $ hg debugsparse -f --exclude 'hide*'
   pending changes to 'hide'
   $ ls -A
   .hg
@@ -254,6 +254,15 @@ 
   hide*
   
 
+Multiple -I and -X can be passed at once
+
+  $ hg debugsparse --reset -I '*2' -X 'hide2'
+  $ ls -A
+  .hg
+  hide.orig
+  show2
+  $ hg debugsparse --reset -X 'hide*'
+
 Verify strip -k resets dirstate correctly
 
   $ hg status
diff --git a/tests/test-sparse-clear.t b/tests/test-sparse-clear.t
--- a/tests/test-sparse-clear.t
+++ b/tests/test-sparse-clear.t
@@ -42,7 +42,7 @@ 
 
 Clear rules when there are excludes
 
-  $ hg debugsparse --exclude *.sparse
+  $ hg debugsparse -X base.sparse -X webpage.sparse
   $ ls -A
   .hg
   data.py
diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -704,21 +704,19 @@ 
 
 def updateconfig(
     repo,
-    pats,
     opts,
-    include=False,
-    exclude=False,
+    *,
+    include=(),
+    exclude=(),
     reset=False,
-    delete=False,
-    enableprofile=False,
-    disableprofile=False,
+    delete=(),
+    enableprofile=(),
+    disableprofile=(),
     force=False,
     usereporootpaths=False,
 ):
     """Perform a sparse config update.
 
-    Only one of the actions may be performed.
-
     The new config is written out and a working directory refresh is performed.
     """
     with repo.wlock(), repo.lock(), repo.dirstate.parentchange():
@@ -736,10 +734,13 @@ 
             newexclude = set(oldexclude)
             newprofiles = set(oldprofiles)
 
-        if any(os.path.isabs(pat) for pat in pats):
-            raise error.Abort(_(b'paths cannot be absolute'))
+        def normalize_pats(pats):
+            if any(os.path.isabs(pat) for pat in pats):
+                raise error.Abort(_(b'paths cannot be absolute'))
 
-        if not usereporootpaths:
+            if usereporootpaths:
+                return pats
+
             # let's treat paths as relative to cwd
             root, cwd = repo.root, repo.getcwd()
             abspats = []
@@ -752,19 +753,20 @@ 
                     abspats.append(ap)
                 else:
                     abspats.append(kindpat)
-            pats = abspats
+            return abspats
 
-        if include:
-            newinclude.update(pats)
-        elif exclude:
-            newexclude.update(pats)
-        elif enableprofile:
-            newprofiles.update(pats)
-        elif disableprofile:
-            newprofiles.difference_update(pats)
-        elif delete:
-            newinclude.difference_update(pats)
-            newexclude.difference_update(pats)
+        include = normalize_pats(include)
+        exclude = normalize_pats(exclude)
+        delete = normalize_pats(delete)
+        disableprofile = normalize_pats(disableprofile)
+        enableprofile = normalize_pats(enableprofile)
+
+        newinclude.difference_update(delete)
+        newexclude.difference_update(delete)
+        newprofiles.difference_update(disableprofile)
+        newinclude.update(include)
+        newprofiles.update(enableprofile)
+        newexclude.update(exclude)
 
         profilecount = len(newprofiles - oldprofiles) - len(
             oldprofiles - newprofiles
diff --git a/hgext/sparse.py b/hgext/sparse.py
--- a/hgext/sparse.py
+++ b/hgext/sparse.py
@@ -76,6 +76,7 @@ 
 from mercurial.i18n import _
 from mercurial.pycompat import setattr
 from mercurial import (
+    cmdutil,
     commands,
     dirstate,
     error,
@@ -153,22 +154,11 @@ 
 
 
 def _clonesparsecmd(orig, ui, repo, *args, **opts):
-    include_pat = opts.get('include')
-    exclude_pat = opts.get('exclude')
-    enableprofile_pat = opts.get('enable_profile')
+    include = opts.get('include')
+    exclude = opts.get('exclude')
+    enableprofile = opts.get('enable_profile')
     narrow_pat = opts.get('narrow')
-    include = exclude = enableprofile = False
-    if include_pat:
-        pat = include_pat
-        include = True
-    if exclude_pat:
-        pat = exclude_pat
-        exclude = True
-    if enableprofile_pat:
-        pat = enableprofile_pat
-        enableprofile = True
-    if sum([include, exclude, enableprofile]) > 1:
-        raise error.Abort(_(b"too many flags specified."))
+
     # if --narrow is passed, it means they are includes and excludes for narrow
     # clone
     if not narrow_pat and (include or exclude or enableprofile):
@@ -176,7 +166,6 @@ 
         def clonesparse(orig, ctx, *args, **kwargs):
             sparse.updateconfig(
                 ctx.repo().unfiltered(),
-                pat,
                 {},
                 include=include,
                 exclude=exclude,
@@ -214,7 +203,7 @@ 
             for pat in pats:
                 dirname, basename = util.split(pat)
                 dirs.add(dirname)
-            sparse.updateconfig(repo, list(dirs), opts, include=True)
+            sparse.updateconfig(repo, opts, include=list(dirs))
         return orig(ui, repo, *pats, **opts)
 
     extensions.wrapcommand(commands.table, b'add', _add)
@@ -286,18 +275,54 @@ 
 @command(
     b'debugsparse',
     [
-        (b'I', b'include', False, _(b'include files in the sparse checkout')),
-        (b'X', b'exclude', False, _(b'exclude files in the sparse checkout')),
-        (b'd', b'delete', False, _(b'delete an include/exclude rule')),
+        (
+            b'I',
+            b'include',
+            [],
+            _(b'include files in the sparse checkout'),
+            _(b'PATTERN'),
+        ),
+        (
+            b'X',
+            b'exclude',
+            [],
+            _(b'exclude files in the sparse checkout'),
+            _(b'PATTERN'),
+        ),
+        (
+            b'd',
+            b'delete',
+            [],
+            _(b'delete an include/exclude rule'),
+            _(b'PATTERN'),
+        ),
         (
             b'f',
             b'force',
             False,
             _(b'allow changing rules even with pending changes'),
         ),
-        (b'', b'enable-profile', False, _(b'enables the specified profile')),
-        (b'', b'disable-profile', False, _(b'disables the specified profile')),
-        (b'', b'import-rules', False, _(b'imports rules from a file')),
+        (
+            b'',
+            b'enable-profile',
+            [],
+            _(b'enables the specified profile'),
+            _(b'PATTERN'),
+        ),
+        (
+            b'',
+            b'disable-profile',
+            [],
+            _(b'disables the specified profile'),
+            _(b'PATTERN'),
+        ),
+        (
+            b'',
+            b'import-rules',
+            [],
+            _(b'imports rules from a file'),
+            _(b'PATTERN'),
+        ),
         (b'', b'clear-rules', False, _(b'clears local include/exclude rules')),
         (
             b'',
@@ -308,10 +333,10 @@ 
         (b'', b'reset', False, _(b'makes the repo full again')),
     ]
     + commands.templateopts,
-    _(b'[--OPTION] PATTERN...'),
+    _(b'[--OPTION]'),
     helpbasic=True,
 )
-def debugsparse(ui, repo, *pats, **opts):
+def debugsparse(ui, repo, **opts):
     """make the current checkout sparse, or edit the existing checkout
 
     The sparse command is used to make the current checkout sparse.
@@ -363,19 +388,13 @@ 
     delete = opts.get(b'delete')
     refresh = opts.get(b'refresh')
     reset = opts.get(b'reset')
-    count = sum(
-        [
-            include,
-            exclude,
-            enableprofile,
-            disableprofile,
-            delete,
-            importrules,
-            refresh,
-            clearrules,
-            reset,
-        ]
+    action = cmdutil.check_at_most_one_arg(
+        opts, b'import_rules', b'clear_rules', b'refresh'
     )
+    updateconfig = bool(
+        include or exclude or delete or reset or enableprofile or disableprofile
+    )
+    count = sum([updateconfig, bool(action)])
     if count > 1:
         raise error.Abort(_(b"too many flags specified"))
 
@@ -397,10 +416,9 @@ 
                 )
             )
 
-    if include or exclude or delete or reset or enableprofile or disableprofile:
+    if updateconfig:
         sparse.updateconfig(
             repo,
-            pats,
             opts,
             include=include,
             exclude=exclude,
@@ -412,7 +430,7 @@ 
         )
 
     if importrules:
-        sparse.importfromfiles(repo, opts, pats, force=force)
+        sparse.importfromfiles(repo, opts, importrules, force=force)
 
     if clearrules:
         sparse.clearrules(repo, force=force)