| Submitter | Gregory Szorc |
|---|---|
| Date | May 16, 2016, 9:24 p.m. |
| Message ID | <a2a87f12649b7485f42b.1463433897@gps-windows-desktop.sfo1.mozilla.com> |
| Download | mbox | patch |
| Permalink | /patch/15151/ |
| State | Accepted |
| Headers | show |
Comments
On Mon, May 16, 2016 at 02:24:57PM -0700, Gregory Szorc wrote: > # HG changeset patch > # User Gregory Szorc <gregory.szorc@gmail.com> > # Date 1463433699 25200 > # Mon May 16 14:21:39 2016 -0700 > # Node ID a2a87f12649b7485f42b4d4d25e8f27e5a0c3f07 > # Parent 90d84e1e427a9d65aedd870cdb7283f84bb30141 > purge: use opts.get() Sure, queued. Thanks! > > Most commands use opts.get() to retrieve values for options > that may not be explicitly passed. purge wasn't. > > This makes it easier to call purge() from 3rd party extensions. > > diff --git a/hgext/purge.py b/hgext/purge.py > --- a/hgext/purge.py > +++ b/hgext/purge.py > @@ -79,44 +79,44 @@ def purge(ui, repo, *dirs, **opts): > If directories are given on the command line, only files in these > directories are considered. > > Be careful with purge, as you could irreversibly delete some files > you forgot to add to the repository. If you only want to print the > list of files that this program would delete, use the --print > option. > ''' > - act = not opts['print'] > + act = not opts.get('print') > eol = '\n' > - if opts['print0']: > + if opts.get('print0'): > eol = '\0' > act = False # --print0 implies --print > - removefiles = opts['files'] > - removedirs = opts['dirs'] > + removefiles = opts.get('files') > + removedirs = opts.get('dirs') > if not removefiles and not removedirs: > removefiles = True > removedirs = True > > def remove(remove_func, name): > if act: > try: > remove_func(repo.wjoin(name)) > except OSError: > m = _('%s cannot be removed') % name > - if opts['abort_on_err']: > + if opts.get('abort_on_err'): > raise error.Abort(m) > ui.warn(_('warning: %s\n') % m) > else: > ui.write('%s%s' % (name, eol)) > > match = scmutil.match(repo[None], dirs, opts) > if removedirs: > directories = [] > match.explicitdir = match.traversedir = directories.append > - status = repo.status(match=match, ignored=opts['all'], unknown=True) > + status = repo.status(match=match, ignored=opts.get('all'), unknown=True) > > if removefiles: > for f in sorted(status.unknown + status.ignored): > if act: > ui.note(_('removing file %s\n') % f) > remove(util.unlink, f) > > if removedirs: > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
This is not the first time we need to do this, should we grow some semi-official API to call a command? (some subset of dispatch with a pythonic way of passing argument) On 05/24/2016 05:00 PM, Augie Fackler wrote: > On Mon, May 16, 2016 at 02:24:57PM -0700, Gregory Szorc wrote: >> # HG changeset patch >> # User Gregory Szorc <gregory.szorc@gmail.com> >> # Date 1463433699 25200 >> # Mon May 16 14:21:39 2016 -0700 >> # Node ID a2a87f12649b7485f42b4d4d25e8f27e5a0c3f07 >> # Parent 90d84e1e427a9d65aedd870cdb7283f84bb30141 >> purge: use opts.get() > > Sure, queued. Thanks! > >> >> Most commands use opts.get() to retrieve values for options >> that may not be explicitly passed. purge wasn't. >> >> This makes it easier to call purge() from 3rd party extensions. >> >> diff --git a/hgext/purge.py b/hgext/purge.py >> --- a/hgext/purge.py >> +++ b/hgext/purge.py >> @@ -79,44 +79,44 @@ def purge(ui, repo, *dirs, **opts): >> If directories are given on the command line, only files in these >> directories are considered. >> >> Be careful with purge, as you could irreversibly delete some files >> you forgot to add to the repository. If you only want to print the >> list of files that this program would delete, use the --print >> option. >> ''' >> - act = not opts['print'] >> + act = not opts.get('print') >> eol = '\n' >> - if opts['print0']: >> + if opts.get('print0'): >> eol = '\0' >> act = False # --print0 implies --print >> - removefiles = opts['files'] >> - removedirs = opts['dirs'] >> + removefiles = opts.get('files') >> + removedirs = opts.get('dirs') >> if not removefiles and not removedirs: >> removefiles = True >> removedirs = True >> >> def remove(remove_func, name): >> if act: >> try: >> remove_func(repo.wjoin(name)) >> except OSError: >> m = _('%s cannot be removed') % name >> - if opts['abort_on_err']: >> + if opts.get('abort_on_err'): >> raise error.Abort(m) >> ui.warn(_('warning: %s\n') % m) >> else: >> ui.write('%s%s' % (name, eol)) >> >> match = scmutil.match(repo[None], dirs, opts) >> if removedirs: >> directories = [] >> match.explicitdir = match.traversedir = directories.append >> - status = repo.status(match=match, ignored=opts['all'], unknown=True) >> + status = repo.status(match=match, ignored=opts.get('all'), unknown=True) >> >> if removefiles: >> for f in sorted(status.unknown + status.ignored): >> if act: >> ui.note(_('removing file %s\n') % f) >> remove(util.unlink, f) >> >> if removedirs: >> _______________________________________________ >> Mercurial-devel mailing list >> Mercurial-devel@mercurial-scm.org >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel >
On Fri, May 27, 2016 at 7:32 AM, Pierre-Yves David < pierre-yves.david@ens-lyon.org> wrote: > This is not the first time we need to do this, should we grow some > semi-official API to call a command? (some subset of dispatch with a > pythonic way of passing argument) > I'm not sure what you have in mind. We do have the argument names from @command. IIRC dispatch already ensures all CLI arguments have named arguments in the function call. Perhaps we'd need to split that out and make an API so extensions can call that? But you can't enforce that since an extension can get a handle on the command function and call it directly. I suppose it's better than nothing. > > On 05/24/2016 05:00 PM, Augie Fackler wrote: > > On Mon, May 16, 2016 at 02:24:57PM -0700, Gregory Szorc wrote: > >> # HG changeset patch > >> # User Gregory Szorc <gregory.szorc@gmail.com> > >> # Date 1463433699 25200 > >> # Mon May 16 14:21:39 2016 -0700 > >> # Node ID a2a87f12649b7485f42b4d4d25e8f27e5a0c3f07 > >> # Parent 90d84e1e427a9d65aedd870cdb7283f84bb30141 > >> purge: use opts.get() > > > > Sure, queued. Thanks! > > > >> > >> Most commands use opts.get() to retrieve values for options > >> that may not be explicitly passed. purge wasn't. > >> > >> This makes it easier to call purge() from 3rd party extensions. > >> > >> diff --git a/hgext/purge.py b/hgext/purge.py > >> --- a/hgext/purge.py > >> +++ b/hgext/purge.py > >> @@ -79,44 +79,44 @@ def purge(ui, repo, *dirs, **opts): > >> If directories are given on the command line, only files in these > >> directories are considered. > >> > >> Be careful with purge, as you could irreversibly delete some files > >> you forgot to add to the repository. If you only want to print the > >> list of files that this program would delete, use the --print > >> option. > >> ''' > >> - act = not opts['print'] > >> + act = not opts.get('print') > >> eol = '\n' > >> - if opts['print0']: > >> + if opts.get('print0'): > >> eol = '\0' > >> act = False # --print0 implies --print > >> - removefiles = opts['files'] > >> - removedirs = opts['dirs'] > >> + removefiles = opts.get('files') > >> + removedirs = opts.get('dirs') > >> if not removefiles and not removedirs: > >> removefiles = True > >> removedirs = True > >> > >> def remove(remove_func, name): > >> if act: > >> try: > >> remove_func(repo.wjoin(name)) > >> except OSError: > >> m = _('%s cannot be removed') % name > >> - if opts['abort_on_err']: > >> + if opts.get('abort_on_err'): > >> raise error.Abort(m) > >> ui.warn(_('warning: %s\n') % m) > >> else: > >> ui.write('%s%s' % (name, eol)) > >> > >> match = scmutil.match(repo[None], dirs, opts) > >> if removedirs: > >> directories = [] > >> match.explicitdir = match.traversedir = directories.append > >> - status = repo.status(match=match, ignored=opts['all'], > unknown=True) > >> + status = repo.status(match=match, ignored=opts.get('all'), > unknown=True) > >> > >> if removefiles: > >> for f in sorted(status.unknown + status.ignored): > >> if act: > >> ui.note(_('removing file %s\n') % f) > >> remove(util.unlink, f) > >> > >> if removedirs: > >> _______________________________________________ > >> Mercurial-devel mailing list > >> Mercurial-devel@mercurial-scm.org > >> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > _______________________________________________ > > Mercurial-devel mailing list > > Mercurial-devel@mercurial-scm.org > > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel > > >
On 06/01/2016 04:35 AM, Gregory Szorc wrote: > > > On Fri, May 27, 2016 at 7:32 AM, Pierre-Yves David > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>> > wrote: > > This is not the first time we need to do this, should we grow some > semi-official API to call a command? (some subset of dispatch with a > pythonic way of passing argument) > > > I'm not sure what you have in mind. We do have the argument names from > @command. IIRC dispatch already ensures all CLI arguments have named > arguments in the function call. Perhaps we'd need to split that out and > make an API so extensions can call that? But you can't enforce that > since an extension can get a handle on the command function and call it > directly. I suppose it's better than nothing. Yes, splitting dispatch so that one can call something like callcommand('commit', 'myfile.txt', message="foobar", user="babar@savannah") And still have all the dispatch logic apply (except for alias/default). Right now, you have to call: dispatch('commit', 'myfile.txt', '--message', "foobar", '--user', "babar@savannah") This is vastly unpythonic. Sure, extension could still grab the handle directly, but they would not have a good reason to do so anymore.
Patch
diff --git a/hgext/purge.py b/hgext/purge.py --- a/hgext/purge.py +++ b/hgext/purge.py @@ -79,44 +79,44 @@ def purge(ui, repo, *dirs, **opts): If directories are given on the command line, only files in these directories are considered. Be careful with purge, as you could irreversibly delete some files you forgot to add to the repository. If you only want to print the list of files that this program would delete, use the --print option. ''' - act = not opts['print'] + act = not opts.get('print') eol = '\n' - if opts['print0']: + if opts.get('print0'): eol = '\0' act = False # --print0 implies --print - removefiles = opts['files'] - removedirs = opts['dirs'] + removefiles = opts.get('files') + removedirs = opts.get('dirs') if not removefiles and not removedirs: removefiles = True removedirs = True def remove(remove_func, name): if act: try: remove_func(repo.wjoin(name)) except OSError: m = _('%s cannot be removed') % name - if opts['abort_on_err']: + if opts.get('abort_on_err'): raise error.Abort(m) ui.warn(_('warning: %s\n') % m) else: ui.write('%s%s' % (name, eol)) match = scmutil.match(repo[None], dirs, opts) if removedirs: directories = [] match.explicitdir = match.traversedir = directories.append - status = repo.status(match=match, ignored=opts['all'], unknown=True) + status = repo.status(match=match, ignored=opts.get('all'), unknown=True) if removefiles: for f in sorted(status.unknown + status.ignored): if act: ui.note(_('removing file %s\n') % f) remove(util.unlink, f) if removedirs: