| Submitter | Siddharth Agarwal |
|---|---|
| Date | Nov. 22, 2014, 1 a.m. |
| Message ID | <959a08bde79e880c8ce3.1416618006@devbig136.prn2.facebook.com> |
| Download | mbox | patch |
| Permalink | /patch/6825/ |
| State | Superseded |
| Commit | 27af986a332bb1c01d16ad8a71e0072532f4bf78 |
| Headers | show |
Comments
On Fri, Nov 21, 2014 at 05:00:06PM -0800, Siddharth Agarwal wrote: > # HG changeset patch > # User Siddharth Agarwal <sid0@fb.com> > # Date 1416359414 28800 > # Tue Nov 18 17:10:14 2014 -0800 > # Node ID 959a08bde79e880c8ce3ed638a5e4f242db6d1a0 > # Parent ec03d9f0198eafe8933924bbeadd2586866eb9f7 > patch.difffeatureopts: add a feature for diff.git > > This deserves to be its own feature -- indeed, this is often the only feature > several commands care about. > > diff --git mercurial/patch.py mercurial/patch.py > --- mercurial/patch.py > +++ mercurial/patch.py > @@ -1559,11 +1559,12 @@ > pass > > def diffallopts(ui, opts=None, untrusted=False, section='diff'): > - return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section) > + return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section, > + git=True) > > diffopts = diffallopts > > -def difffeatureopts(ui, opts=None, untrusted=False, section='diff'): > +def difffeatureopts(ui, opts=None, untrusted=False, section='diff', git=False): This function is begging for a docstring that describes what the various options mean. In this change it's still kind of guessable, but a couple of patches later it becomes pretty subtle. > def get(key, name=None, getter=ui.configbool, forceplain=None): > if opts: > v = opts.get(key) > @@ -1575,7 +1576,6 @@ > > buildopts = { > 'text': opts and opts.get('text'), > - 'git': get('git'), > 'nodates': get('nodates'), > 'nobinary': get('nobinary'), > 'noprefix': get('noprefix', forceplain=False), > @@ -1586,6 +1586,9 @@ > 'context': get('unified', getter=ui.config), > } > > + if git: > + buildopts['git'] = get('git') > + > return mdiff.diffopts(**buildopts) > > def diff(repo, node1=None, node2=None, match=None, changes=None, opts=None, > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@selenic.com > http://selenic.com/mailman/listinfo/mercurial-devel
On 11/25/2014 07:36 AM, Augie Fackler wrote: > On Fri, Nov 21, 2014 at 05:00:06PM -0800, Siddharth Agarwal wrote: >> # HG changeset patch >> # User Siddharth Agarwal <sid0@fb.com> >> # Date 1416359414 28800 >> # Tue Nov 18 17:10:14 2014 -0800 >> # Node ID 959a08bde79e880c8ce3ed638a5e4f242db6d1a0 >> # Parent ec03d9f0198eafe8933924bbeadd2586866eb9f7 >> patch.difffeatureopts: add a feature for diff.git >> >> This deserves to be its own feature -- indeed, this is often the only feature >> several commands care about. >> >> diff --git mercurial/patch.py mercurial/patch.py >> --- mercurial/patch.py >> +++ mercurial/patch.py >> @@ -1559,11 +1559,12 @@ >> pass >> >> def diffallopts(ui, opts=None, untrusted=False, section='diff'): >> - return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section) >> + return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section, >> + git=True) >> >> diffopts = diffallopts >> >> -def difffeatureopts(ui, opts=None, untrusted=False, section='diff'): >> +def difffeatureopts(ui, opts=None, untrusted=False, section='diff', git=False): > This function is begging for a docstring that describes what the > various options mean. In this change it's still kind of guessable, but > a couple of patches later it becomes pretty subtle. Would you prefer a V2 or a followup?
On Tue, Nov 25, 2014 at 1:39 PM, Siddharth Agarwal <sid@less-broken.com> wrote: > On 11/25/2014 07:36 AM, Augie Fackler wrote: >> >> On Fri, Nov 21, 2014 at 05:00:06PM -0800, Siddharth Agarwal wrote: >>> >>> # HG changeset patch >>> # User Siddharth Agarwal <sid0@fb.com> >>> # Date 1416359414 28800 >>> # Tue Nov 18 17:10:14 2014 -0800 >>> # Node ID 959a08bde79e880c8ce3ed638a5e4f242db6d1a0 >>> # Parent ec03d9f0198eafe8933924bbeadd2586866eb9f7 >>> patch.difffeatureopts: add a feature for diff.git >>> >>> This deserves to be its own feature -- indeed, this is often the only >>> feature >>> several commands care about. >>> >>> diff --git mercurial/patch.py mercurial/patch.py >>> --- mercurial/patch.py >>> +++ mercurial/patch.py >>> @@ -1559,11 +1559,12 @@ >>> pass >>> >>> def diffallopts(ui, opts=None, untrusted=False, section='diff'): >>> - return difffeatureopts(ui, opts=opts, untrusted=untrusted, >>> section=section) >>> + return difffeatureopts(ui, opts=opts, untrusted=untrusted, >>> section=section, >>> + git=True) >>> >>> diffopts = diffallopts >>> >>> -def difffeatureopts(ui, opts=None, untrusted=False, section='diff'): >>> +def difffeatureopts(ui, opts=None, untrusted=False, section='diff', >>> git=False): >> >> This function is begging for a docstring that describes what the >> various options mean. In this change it's still kind of guessable, but >> a couple of patches later it becomes pretty subtle. > > > Would you prefer a V2 or a followup? A v2 would be a little easier for me at this point.
Patch
diff --git mercurial/patch.py mercurial/patch.py --- mercurial/patch.py +++ mercurial/patch.py @@ -1559,11 +1559,12 @@ pass def diffallopts(ui, opts=None, untrusted=False, section='diff'): - return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section) + return difffeatureopts(ui, opts=opts, untrusted=untrusted, section=section, + git=True) diffopts = diffallopts -def difffeatureopts(ui, opts=None, untrusted=False, section='diff'): +def difffeatureopts(ui, opts=None, untrusted=False, section='diff', git=False): def get(key, name=None, getter=ui.configbool, forceplain=None): if opts: v = opts.get(key) @@ -1575,7 +1576,6 @@ buildopts = { 'text': opts and opts.get('text'), - 'git': get('git'), 'nodates': get('nodates'), 'nobinary': get('nobinary'), 'noprefix': get('noprefix', forceplain=False), @@ -1586,6 +1586,9 @@ 'context': get('unified', getter=ui.config), } + if git: + buildopts['git'] = get('git') + return mdiff.diffopts(**buildopts) def diff(repo, node1=None, node2=None, match=None, changes=None, opts=None,