D4125: narrow: add '--extend' flag to tracked command

[martinvonz (Martin von Zweigbergk)]

martinvonz added inline comments.

INLINE COMMENTS

> narrowcommands.py:303
>       ('', 'addexclude', [], _('new paths to exclude')),
> +     ('', 'extend', '', _('file to read to extend the narrowspec')),
>       ('', 'removeexclude', [], _('old paths to no longer exclude')),

nit: `--extend` may be too generic. How about `--addfromfile`? Can be done in a follow-up if you agree.

> narrowcommands.py:352
> +        try:
> +            fdata = repo[None][extend].data()
> +        except IOError:

Why is it required to be in the working directory? Why not accept any kind of path? I don't know if it's intentional or not, but I think this also means that `cd dir; hg tracked --extend foo` will attempt to read `foo` from the repo root and not from the current directory, which may be confusing to users.

> narrowcommands.py:360
> +            raise error.Abort(_("including other spec files using '%include' "
> +                                "is not supported in narrowspec"))
> +        opts['addinclude'].extend(list(includepats))

could you send a patch to fix the typoed "supported" on narrowspec.py too?

> narrowcommands.py:361-362
> +                                "is not supported in narrowspec"))
> +        opts['addinclude'].extend(list(includepats))
> +        opts['addexclude'].extend(list(excludepats))
> +

nit: is the `list` conversion necessary?

> test-narrow-trackedcmd.t:3-6
> +  $ cat << EOF >> $HGRCPATH
> +  > [experimental]
> +  > treemanifest = 1
> +  > EOF

are treemanifests relevant to this test?

> test-narrow-trackedcmd.t:52-86
> +add more upstream files which we will include in a wider narrow spec
> +
> +  $ cd master
> +
> +  $ mkdir wider
> +  $ echo 'wider' > wider/f
> +  $ hg add wider/f

any reason not to add these from the beginning (i.e. before creating the clone)?

> test-narrow-trackedcmd.t:91
> +  $ cd narrow
> +  $ hg tracked --extend
> +  hg tracked: option --extend requires argument

This is part of why I don't like `--extend`: it sounds like it can be given without a parameter

> test-narrow-trackedcmd.t:118
> +
> +  $ cat >> specs <<EOF
> +  > %include foo

nit: s/>>/>/

REPOSITORY
  rHG Mercurial

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

To: pulkit, durin42, #hg-reviewers
Cc: martinvonz, mercurial-devel