<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jul 29, 2015 at 1:14 PM, Pierre-Yves David <span dir="ltr"><<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
<br>
On 07/21/2015 12:06 PM, Matt Mackall wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Mon, 2015-07-20 at 14:13 -0700, Gregory Szorc wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Mon, Jul 20, 2015 at 1:07 PM, Matt Mackall <<a href="mailto:mpm@selenic.com" target="_blank">mpm@selenic.com</a>> wrote:<br>
<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Mon, 2015-07-20 at 20:18 +0900, Yuya Nishihara wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
On Sat, 18 Jul 2015 14:24:30 -0700, Gregory Szorc wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
# HG changeset patch<br>
# User Gregory Szorc <<a href="mailto:gregory.szorc@gmail.com" target="_blank">gregory.szorc@gmail.com</a>><div><div class="h5"><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<br>
</div></div></blockquote></blockquote><div><div class="h5">
matcher"""<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+    def diff(self, ctx2=None, match=None, allowgit=True,<br>
</blockquote></blockquote>
allowwhitespace=True,<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+             allowformatchanging=True, **opts):<br>
+        """Returns a diff generator for the given contexts and<br>
</blockquote></blockquote>
matcher.<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+<br>
+        The various allow* arguments control whether the diff<br>
</blockquote></blockquote>
features under<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+        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>
+<br>
</blockquote></blockquote>
  formatchanging=allowformatchanging)<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
          return patch.diff(self._repo, ctx2, self, match=match,<br>
</blockquote></blockquote>
opts=diffopts)<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I'm noob about this API, but I think patch.diff*opts() exists to pack<br>
</blockquote>
various<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
diff-related options into one argument. It seems this change goes the<br>
</blockquote>
opposite<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
direction.<br>
</blockquote>
<br>
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>
<br>
</div></div></blockquote><div><div class="h5">
<br>
Sorry. I found this last week and I considered it stable worthy because it<br>
prevents bustage of {diffstat}.<br>
</div></div></blockquote><div><div class="h5">
<br>
I'm not saying fixing this issue is not stable-worthy. I'm saying any<br>
sort of significant moving of stuff around to make the fix pretty is not<br>
how we do things in stable. We only care about user benefit/churn and<br>
"pretty" is bad for that equation.<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Unfortunately, this patch breaks on filenames with spaces. (This is also a<br>
deficiency of the Git patch format.)<br>
</blockquote>
<br>
Do we not have any of those in the test suite? Because my change passes<br>
tests. But also, if we're generating git diffs that aren't parseable by<br>
git.. we have a bigger problem.<br>
</div></div></blockquote>
<br>
So, what is the general state of this ?<br>
<br>
1) did you had a stable worthy issue?<br>
2) if so was it fixed on stable or should we expect a fix in the next 48h? (urg:-/)<br>
3) What should I mark this series on patchwork?<span class="HOEnZb"><font color="#888888"></font></span></blockquote><div><br></div><div>While I think this bug fix is stable worthy, I have little to no inclination to restructure this in the next 48 hours. I'm not exactly sure what qualifies as a stable-appropriate patch and I'd rather not incur the extra work to refactor this before and after 3.5. I'll just send again after the freeze. Although, it sounds like there may be more discussion about the general approach. So I dunno.<br></div></div></div></div>