[PATCH 1 of 4] largefiles: don't pop largefile specific arguments to the add command

[Martin von Zweigbergk]

At first I had assumed that it was protected from reaching that piece of
code without 'normal' present, but that doesn't seem to be the case. I
would have assumed, like you seem to have, that opts['normal'] would only
present if --normal was given on the command line, but take a look at
uisetup.py, which seems to register extra options to the command. I haven't
checked, but I would guess the third option there is the default (None in
this case), which some early piece of code (dispatch.py?) must read and
create opts based on that and the arguments given on the command line.
Mostly guessing here, though.

On Thu Jan 15 2015 at 9:11:11 PM Matt Harbison <mharbison72 at gmail.com>
wrote:

> On Thu, 15 Jan 2015 23:36:56 -0500, Martin von Zweigbergk
> <martinvonz at google.com> wrote:
>
> > On Thu Jan 15 2015 at 7:40:19 PM Matt Harbison <mharbison72 at gmail.com>
> > wrote:
> >
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison at yahoo.com>
> >> # Date 1420068272 18000
> >> #      Wed Dec 31 18:24:32 2014 -0500
> >> # Node ID 6c893ee9c85def62e2f19de9f4a2b5288a12a5fd
> >> # Parent  049a9e3a078d7c988cb12ed456aad6ec2779ea69
> >> largefiles: don't pop largefile specific arguments to the add command
> >>
> >> The arguments will need to stay present when making add work with
> >> subrepos.
> >>
> >> diff --git a/hgext/largefiles/overrides.py
> >> b/hgext/largefiles/overrides.py
> >> --- a/hgext/largefiles/overrides.py
> >> +++ b/hgext/largefiles/overrides.py
> >> @@ -90,9 +90,9 @@
> >>              scmutil.matchandpats)
> >>
> >>  def addlargefiles(ui, repo, isaddremove, matcher, **opts):
> >> -    large = opts.pop('large', None)
> >> +    large = opts.get('large')
> >>      lfsize = lfutil.getminsize(
> >> -        ui, lfutil.islfilesrepo(repo), opts.pop('lfsize', None))
> >> +        ui, lfutil.islfilesrepo(repo), opts.get('lfsize'))
> >>
> >>      lfmatcher = None
> >>      if lfutil.islfilesrepo(repo):
> >> @@ -247,7 +247,7 @@
> >>  # matcher which matches only the normal files and runs the original
> >>  # version of add.
> >>  def overrideadd(orig, ui, repo, *pats, **opts):
> >> -    normal = opts.pop('normal')
> >> +    normal = opts.get('normal')
> >>
> >
> > Nit: unlike pop(), get() doesn't raise KeyError. Use opts['normal']
> > unless
> > that's a wanted change.
>
> Hmm, you're right.  I have no idea how this was working then, because
> --normal doesn't have to be specified on the command line.  This is the
> first line of the commands.add() override, so nothing command specific is
> adding it.  The lack of a KeyError implies it is in there, but how?
> Conversely, if --large is specified on the command line, it doesn't
> complain that both options can't be specified, so it doesn't actually seem
> like it is there.  debugpathcomplete also accesses its args with [], but
> that's the only other case I see.  Any ideas?
>
> The short answer though is, I don't think we want to raise a KeyError when
> the user doesn't specify an optional argument.
>
> --Matt
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150116/e1015634/attachment.html>