Use Patches.make_name() in various commands

The new Patches.make_name() method replaces various uses of the
utils.make_patch_name() and utils.find_patch_name() functions, which are
removed. make_name() is expected to be more robust at ensuring valid and
unique patch names.

Signed-off-by: Peter Grayson <[email protected]>

Author: jpgrayson

stgit/commands/new.py

@@ -95,8 +95,7 @@ def func(parser, options, args):
         cd = run_commit_msg_hook(stack.repository, cd)
 
     if name is None:
-        name = utils.make_patch_name(cd.message_str, stack.patches.exists)
-        assert stack.patches.is_name_valid(name)
+        name = stack.patches.make_name(cd.message_str)
 
     # Write the new patch.
     stack.repository.default_iw

stgit/commands/pick.py

@@ -15,7 +15,7 @@
 from stgit.lib.transaction import StackTransaction, TransactionHalted
 from stgit.out import out
 from stgit.run import Run
-from stgit.utils import STGIT_CONFLICT, STGIT_SUCCESS, find_patch_name, make_patch_name
+from stgit.utils import STGIT_CONFLICT, STGIT_SUCCESS
 
 __copyright__ = """
 Copyright (C) 2005, Catalin Marinas <[email protected]>
@@ -120,9 +120,9 @@ def __pick_commit(stack, ref_stack, iw, commit, patchname, options):
         patchname = 'revert-' + patchname
 
     if patchname:
-        patchname = find_patch_name(patchname, stack.patches.exists)
+        patchname = stack.patches.make_name(patchname, lower=False)
     else:
-        patchname = make_patch_name(commit.data.message_str, stack.patches.exists)
+        patchname = stack.patches.make_name(commit.data.message_str)
 
     if options.parent:
         parent = git_commit(options.parent, repository, ref_stack.name)

stgit/commands/refresh.py

@@ -213,7 +213,7 @@ def make_temp_patch(stack, patch_name, tree):
             message='Refresh of %s' % patch_name,
         )
     )
-    temp_name = utils.make_patch_name('refresh-temp', stack.patches.exists)
+    temp_name = stack.patches.make_name('refresh-temp')
     trans = StackTransaction(stack, 'refresh (create temporary patch)')
     trans.patches[temp_name] = commit
     trans.applied.append(temp_name)

stgit/commands/repair.py

@@ -1,7 +1,6 @@
 from stgit.commands.common import CmdException, DirectoryGotoTopLevel
 from stgit.lib.transaction import StackTransaction, TransactionHalted
 from stgit.out import out
-from stgit.utils import make_patch_name
 
 __copyright__ = """
 Copyright (C) 2006, Karl Hasselström <[email protected]>
@@ -129,10 +128,7 @@ def func(parser, options, args):
         )
 
         for c in patchify:
-            pn = make_patch_name(
-                c.data.message_str,
-                unacceptable=lambda name: any(p.name == name for p in patches),
-            )
+            pn = stack.patches.make_name(c.data.message_str)
             out.info('Creating patch %s from commit %s' % (pn, c.sha1))
             applied.append(stack.patches.new(pn, c, 'repair'))
         out.done()

stgit/commands/squash.py

@@ -117,14 +117,11 @@ def _squash_patches(trans, patches, msg, save_template, no_verify=False):
 
 def _squash(stack, iw, name, msg, save_template, patches, no_verify=False):
     # If a name was supplied on the command line, make sure it's OK.
-    def bad_name(pn):
-        return pn not in patches and stack.patches.exists(pn)
+    if name and name not in patches and stack.patches.exists(name):
+        raise CmdException('Patch name "%s" already taken' % name)
 
     def get_name(cd):
-        return name or utils.make_patch_name(cd.message_str, bad_name)
-
-    if name and bad_name(name):
-        raise CmdException('Patch name "%s" already taken' % name)
+        return name or stack.patches.make_name(cd.message_str, allow=patches)
 
     def make_squashed_patch(trans, new_commit_data):
         name = get_name(new_commit_data)
@@ -172,6 +169,8 @@ def func(parser, options, args):
     patches = parse_patches(args, list(stack.patchorder.all))
     if len(patches) < 2:
         raise CmdException('Need at least two patches')
+    if options.name and not stack.patches.is_name_valid(options.name):
+        raise CmdException('Patch name "%s" is invalid' % options.name)
     return _squash(
         stack,
         stack.repository.default_iw,

stgit/commands/uncommit.py

@@ -1,4 +1,3 @@
-from stgit import utils
 from stgit.argparse import opt
 from stgit.commands.common import CmdException, DirectoryHasRepository
 from stgit.lib import transaction
@@ -143,11 +142,13 @@ def check_and_append(c, n):
         for pn in patchnames:
             if pn in taken_names:
                 raise CmdException('Patch name "%s" already taken' % pn)
+            elif not stack.patches.is_name_valid(pn):
+                raise CmdException('Patch name "%s" is invalid' % pn)
             taken_names.add(pn)
     else:
         patchnames = []
         for c in reversed(commits):
-            pn = utils.make_patch_name(c.data.message_str, lambda pn: pn in taken_names)
+            pn = stack.patches.make_name(c.data.message_str, disallow=taken_names)
             patchnames.append(pn)
             taken_names.add(pn)
         patchnames.reverse()

stgit/lib/stack.py

@@ -168,7 +168,7 @@ def new(self, name, commit, msg):
         self._patches[name] = p
         return p
 
-    def make_name(self, raw, unique=True, lower=True):
+    def make_name(self, raw, unique=True, lower=True, allow=(), disallow=()):
         """Make a unique and valid patch name from provided raw name.
 
         The raw name may come from a filename, commit message, or email subject line.
@@ -228,7 +228,9 @@ def make_name(self, raw, unique=True, lower=True):
             return short_name
 
         unique_name = short_name
-        while self.exists(unique_name):
+        while unique_name not in allow and (
+            self.exists(unique_name) or unique_name in disallow
+        ):
             m = re.match(r'(.*?)(-)?(\d+)$', unique_name)
             if m:
                 base, sep, n_str = m.groups()

stgit/utils.py

@@ -124,54 +124,6 @@ def edit_bytes(s, filename):
     return s
 
 
-def find_patch_name(patchname, unacceptable):
-    """Find a patch name which is acceptable."""
-    if unacceptable(patchname):
-        suffix = 0
-        while unacceptable('%s-%d' % (patchname, suffix)):
-            suffix += 1
-        patchname = '%s-%d' % (patchname, suffix)
-    return patchname
-
-
-def patch_name_from_msg(msg):
-    """Return a string to be used as a patch name.
-
-    This is generated from the first line of the specified message string.
-
-    """
-    if not msg:
-        return None
-
-    name_len = config.getint('stgit.namelength')
-
-    subject_line = msg.split('\n', 1)[0].lstrip().lower()
-    words = re.sub(r'(?u)[\W]+', ' ', subject_line).split()
-
-    # use loop to avoid truncating the last name
-    name = words and words[0] or 'unknown'
-    for word in words[1:]:
-        new = name + '-' + word
-        if len(new) > name_len:
-            break
-        name = new
-
-    return name
-
-
-def make_patch_name(msg, unacceptable, default_name='patch'):
-    """Generate a patch name from the given commit message.
-
-    The generated name is guaranteed to make unacceptable(name) be false. If the commit
-    message is empty, base the name on default_name instead.
-
-    """
-    patchname = patch_name_from_msg(msg)
-    if not patchname:
-        patchname = default_name
-    return find_patch_name(patchname, unacceptable)
-
-
 def add_trailer(message, trailer, name, email):
     trailer_line = '%s: %s <%s>' % (trailer, name, email)
     return (

t/t1300-uncommit.sh

@@ -122,6 +122,10 @@ test_expect_success 'Attempt to reuse patch name' '
   stg commit --all
 '
 
+test_expect_success 'Attempt to use invalid patch name' '
+  command_error stg uncommit bad..patchname
+'
+
 test_expect_success 'Uncommit a commit with not precisely one parent' '
     command_error stg uncommit -n 5  &&
     [ "$(echo $(stg series))" = "" ]

t/t2600-squash.sh

@@ -25,6 +25,11 @@ test_expect_success 'Attempt duplicate patch name' '
     grep -e "Patch name \"p3\" already taken"
 '
 
+test_expect_success 'Attempt invalid patch name' '
+    command_error stg squash -n invalid..name -- p0 p1 2>&1 |
+    grep -e "Patch name \"invalid..name\" is invalid"
+'
+
 test_expect_success 'Save template' '
     stg squash --save-template mytemplate p0 p1 &&
     test_path_is_file mytemplate &&