Patchwork [2,of,2] shelve: choose a legal shelve name when no name is passed (issue5112)

# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1482415530 -19800
#      Thu Dec 22 19:35:30 2016 +0530
# Node ID 79fe51eaad172888e8d59431743d6fa466e0dfc9
# Parent  04e469e02e98727d43175b621b675404a4aa0013
shelve: choose a legal shelve name when no name is passed (issue5112)

Currently if our branch name contains '\' or starts with '.', shelve chooses
an illegal shelve name. This behaviour is not good as it itself is choosing
something which it won't accept further. We can raise errors if user passes
a name which is illegal.
After this patch, if '\' is contained in branch name or bookmark name, it will
be replaced by '_' while choosing a shelve name and if they starts with '.',
the first '.' is replaced by '_'.

On Fri, Dec 23, 2016 at 12:21:45AM +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1482415530 -19800
> #      Thu Dec 22 19:35:30 2016 +0530
> # Node ID 79fe51eaad172888e8d59431743d6fa466e0dfc9
> # Parent  04e469e02e98727d43175b621b675404a4aa0013
> shelve: choose a legal shelve name when no name is passed (issue5112)

This is queued, based on the following assumption: if I try to produce
a shelve named default\pony and another one named default_pony, bad
things won't happen. Is that correct?

Thanks!

>
> Currently if our branch name contains '\' or starts with '.', shelve chooses
> an illegal shelve name. This behaviour is not good as it itself is choosing
> something which it won't accept further. We can raise errors if user passes
> a name which is illegal.
> After this patch, if '\' is contained in branch name or bookmark name, it will
> be replaced by '_' while choosing a shelve name and if they starts with '.',
> the first '.' is replaced by '_'.
>
> diff -r 04e469e02e98 -r 79fe51eaad17 hgext/shelve.py
> --- a/hgext/shelve.py	Thu Dec 22 23:27:32 2016 +0530
> +++ b/hgext/shelve.py	Thu Dec 22 19:35:30 2016 +0530
> @@ -265,11 +265,22 @@
>      label = repo._activebookmark or parent.branch() or 'default'
>      # slashes aren't allowed in filenames, therefore we rename it
>      label = label.replace('/', '_')
> +    label = label.replace('\\', '_')
> +    # filenames must not start with '.' as it should not be hidden
> +    if label.startswith('.'):
> +        label = label.replace('.', '_', 1)
>
>      if name:
>          if shelvedfile(repo, name, patchextension).exists():
>              e = _("a shelved change named '%s' already exists") % name
>              raise error.Abort(e)
> +
> +        # ensure we are not creating a subdirectory or a hidden file
> +        if '/' in name or '\\' in name:
> +            raise error.Abort(_('shelved change names can not contain slashes'))
> +        if name.startswith('.'):
> +            raise error.Abort(_("shelved change names can not start with '.'"))
> +
>      else:
>          for n in gennames():
>              if not shelvedfile(repo, n, patchextension).exists():
> @@ -278,11 +289,6 @@
>          else:
>              raise error.Abort(_("too many shelved changes named '%s'") % label)
>
> -    # ensure we are not creating a subdirectory or a hidden file
> -    if '/' in name or '\\' in name:
> -        raise error.Abort(_('shelved change names may not contain slashes'))
> -    if name.startswith('.'):
> -        raise error.Abort(_("shelved change names may not start with '.'"))
>      return name
>
>  def mutableancestors(ctx):
> diff -r 04e469e02e98 -r 79fe51eaad17 tests/test-shelve.t
> --- a/tests/test-shelve.t	Thu Dec 22 23:27:32 2016 +0530
> +++ b/tests/test-shelve.t	Thu Dec 22 19:35:30 2016 +0530
> @@ -106,13 +106,13 @@
>  when we are given a name
>
>    $ hg shelve -n foo/bar
> -  abort: shelved change names may not contain slashes
> +  abort: shelved change names can not contain slashes
>    [255]
>    $ hg shelve -n .baz
> -  abort: shelved change names may not start with '.'
> +  abort: shelved change names can not start with '.'
>    [255]
>    $ hg shelve -n foo\\bar
> -  abort: shelved change names may not contain slashes
> +  abort: shelved change names can not contain slashes
>    [255]
>
>  when shelve has to choose itself
> @@ -125,13 +125,13 @@
>    $ hg branch .x -q
>    $ hg commit -q -m "Branch commit 1"
>    $ hg shelve
> -  abort: shelved change names may not start with '.'
> -  [255]
> +  nothing changed
> +  [1]
>    $ hg branch x\\y -q
>    $ hg commit -q -m "Branch commit 2"
>    $ hg shelve
> -  abort: shelved change names may not contain slashes
> -  [255]
> +  nothing changed
> +  [1]
>
>  cleaning the branches made for name checking tests
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

On Sun, Dec 25, 2016 at 1:14 AM, Augie Fackler <raf@durin42.com> wrote:
> On Fri, Dec 23, 2016 at 12:21:45AM +0530, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1482415530 -19800
>> #      Thu Dec 22 19:35:30 2016 +0530
>> # Node ID 79fe51eaad172888e8d59431743d6fa466e0dfc9
>> # Parent  04e469e02e98727d43175b621b675404a4aa0013
>> shelve: choose a legal shelve name when no name is passed (issue5112)
>
> This is queued, based on the following assumption: if I try to produce
> a shelve named default\pony and another one named default_pony, bad
> things won't happen. Is that correct?

Yeah it is, we will get default_pony-01 as our code take care of duplicates.

> On Dec 24, 2016, at 2:50 PM, Pulkit Goyal <7895pulkit@gmail.com> wrote:
> 
> On Sun, Dec 25, 2016 at 1:14 AM, Augie Fackler <raf@durin42.com> wrote:
>> On Fri, Dec 23, 2016 at 12:21:45AM +0530, Pulkit Goyal wrote:
>>> # HG changeset patch
>>> # User Pulkit Goyal <7895pulkit@gmail.com>
>>> # Date 1482415530 -19800
>>> #      Thu Dec 22 19:35:30 2016 +0530
>>> # Node ID 79fe51eaad172888e8d59431743d6fa466e0dfc9
>>> # Parent  04e469e02e98727d43175b621b675404a4aa0013
>>> shelve: choose a legal shelve name when no name is passed (issue5112)
>> 
>> This is queued, based on the following assumption: if I try to produce
>> a shelve named default\pony and another one named default_pony, bad
>> things won't happen. Is that correct?
> 
> Yeah it is, we will get default_pony-01 as our code take care of duplicates.

Shiny. Thanks!