<div dir="ltr">On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <span dir="ltr"><<a href="mailto:mpm@selenic.com" target="_blank">mpm@selenic.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Mon, 2015-07-20 at 20:18 +0900, Yuya Nishihara wrote:<br>
> On Sat, 18 Jul 2015 14:24:30 -0700, Gregory Szorc wrote:<br>
> > # HG changeset patch<br>
> > # User Gregory Szorc <<a href="mailto:gregory.szorc@gmail.com">gregory.szorc@gmail.com</a>><br>
> > # Date 1437254506 25200<br>
> > # Sat Jul 18 14:21:46 2015 -0700<br>
> > # Node ID b6ab3a470c0196159e8931c8c6dd4b5c4a52a4f6<br>
> > # Parent d88519184df5f9538b4f9294bee7a595d5dce6d2<br>
> > context: ability to manipulate diff feature opt-ins<br>
> ><br>
> > Before this patch, basectx.diff respected all diff options. Sometimes<br>
> > consumers want to opt out of certain classes of diff features.<br>
> ><br>
> > Change the function to call patch.difffeatureopts instead of<br>
> > diffallopts and allow the various features to be opted out of via<br>
> > arguments.<br>
> ><br>
> > diff --git a/mercurial/context.py b/mercurial/context.py<br>
> > --- a/mercurial/context.py<br>
> > +++ b/mercurial/context.py<br>
> > @@ -268,15 +268,23 @@ class basectx(object):<br>
> > include, exclude, default,<br>
> > auditor=r.auditor, ctx=self,<br>
> > listsubrepos=listsubrepos, badfn=badfn)<br>
> ><br>
> > - def diff(self, ctx2=None, match=None, **opts):<br>
> > - """Returns a diff generator for the given contexts and matcher"""<br>
> > + def diff(self, ctx2=None, match=None, allowgit=True, allowwhitespace=True,<br>
> > + allowformatchanging=True, **opts):<br>
> > + """Returns a diff generator for the given contexts and matcher.<br>
> > +<br>
> > + The various allow* arguments control whether the diff features under<br>
> > + that category are respected. See patch.difffeatureopts.<br>
> > + """<br>
> > if ctx2 is None:<br>
> > ctx2 = self.p1()<br>
> > if ctx2 is not None:<br>
> > ctx2 = self._repo[ctx2]<br>
> > - diffopts = patch.diffopts(self._repo.ui, opts)<br>
> > + diffopts = patch.difffeatureopts(self._repo.ui, opts=opts,<br>
> > + git=allowgit,<br>
> > + whitespace=allowwhitespace,<br>
> > + formatchanging=allowformatchanging)<br>
> > return patch.diff(self._repo, ctx2, self, match=match, opts=diffopts)<br>
><br>
> I'm noob about this API, but I think patch.diff*opts() exists to pack various<br>
> diff-related options into one argument. It seems this change goes the opposite<br>
> direction.<br>
<br>
</div></div>Yep. Also, it bears repeating: I really don't want _patch series_ on<br>
stable. The explicit goal of stable is to maximize benefit/churn. If you<br>
can't fix something in stable in one simple patch, it may be too much<br>
churn. I don't see any reason why our diffstat code should be picky<br>
about prefixes, so I think we should instead teach it that prefixes are<br>
optional:<br></blockquote><div><br></div><div>Sorry. I found this last week and I considered it stable worthy because it prevents bustage of {diffstat}. I went with the more involved fix because, well, it felt more proper than a hack that would only get refactored post 3.5 anyway.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
diff -r f8aead51aec0 mercurial/patch.py<br>
--- a/mercurial/patch.py Sun Jul 19 18:11:18 2015 +0200<br>
+++ b/mercurial/patch.py Mon Jul 20 15:02:19 2015 -0500<br>
@@ -15,7 +15,7 @@<br>
import base85, mdiff, scmutil, util, diffhelpers, copies, encoding, error<br>
import pathutil<br>
<br>
-gitre = re.compile('diff --git a/(.*) b/(.*)')<br>
+gitre = re.compile('diff --git (?:a/)(.*) (?:b/)(.*)')<br>
tabsplitter = re.compile(r'(\t+|[^\t]+)')<br>
<br>
class PatchError(Exception):<br>
@@ -324,7 +324,7 @@<br>
gitpatches = []<br>
for line in lr:<br>
line = line.rstrip(' \r\n')<br>
- if line.startswith('diff --git a/'):<br>
+ if line.startswith('diff --git '):<br>
m = gitre.match(line)<br>
if m:<br>
if gp:<br>
@@ -814,7 +814,7 @@<br>
class header(object):<br>
"""patch header<br>
"""<br>
- diffgit_re = re.compile('diff --git a/(.*) b/(.*)$')<br>
+ diffgit_re = re.compile('diff --git (?:a/)(.*) (?:b/)(.*)$')<br>
diff_re = re.compile('diff -r .* (.*)$')<br>
allhunks_re = re.compile('(?:index|deleted file) ')<br>
pretty_re = re.compile('(?:new file|deleted file) ')<br>
@@ -1752,7 +1752,7 @@<br>
emitfile = False<br>
yield 'file', (afile, bfile, h, gp and gp.copy() or None)<br>
yield 'hunk', h<br>
- elif x.startswith('diff --git a/'):<br>
+ elif x.startswith('diff --git '):<br>
m = gitre.match(x.rstrip(' \r\n'))<br>
if not m:<br>
continue<br>
@@ -2476,7 +2476,7 @@<br>
addresult()<br>
# set numbers to 0 anyway when starting new file<br>
adds, removes, isbinary = 0, 0, False<br>
- if line.startswith('diff --git a/'):<br>
+ if line.startswith('diff --git '):<br>
filename = gitre.search(line).group(2)<br>
elif line.startswith('diff -r'):<br>
# format: "diff -r ... -r ... filename"<br>
<br>
This patch may go a little too far since I just did a search and<br>
replace, but we should probably also teach the patcher how to apply<br>
prefixless patches.<span class="HOEnZb"><font color="#888888"><br></font></span></blockquote><div><br></div><div>Unfortunately, this patch breaks on filenames with spaces. (This is also a deficiency of the Git patch format.) <br></div></div><br></div></div>