<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[183030] trunk/Tools</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/183030">183030</a></dd>
<dt>Author</dt> <dd>matthew_hanson@apple.com</dd>
<dt>Date</dt> <dd>2015-04-20 15:05:52 -0700 (Mon, 20 Apr 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>prepare-ChangeLog should ignore the preceeding function when processing the removal of a function.
https://bugs.webkit.org/show_bug.cgi?id=143897

Reviewed by David Kilzer.

This is a speculative fix that addresses two issues:

1. An off-by-one error which allowed ending lines to be less than starting lines when a hunk was a pure delete.
We were determining ending lines from combined diffs using the logic: End = Start + Offset - 1.

So for a hunk like &quot;@@ -723,10 +721,0 @@ bool foobar&quot;, we were generating the following starting/ending line pairs:
Before: (723, 729)
After: (721, 720)

Before is correct, but After should be (721, 721), since it represents the beginning and ending lines for the hunk.
Whether there are zero lines or one line in the hunk, the starting and ending line are the same.

This error was causing bad behavior on purely additive and purely subtractive hunks, but since we only refer
to After when generating ChangeLog output, the extractLineRangeBeforeChange had no visible effect on program output.

The fix is to set End to the max of Start + Offset - 1 and Start, rather than always using the former.

2. Creating git diffs from HEAD and not origin/master by default.

Hard-coding origin/master into the originalFile command has the disadvantage of causing the diff to fail entirely
when origin/master does not exist, and to do the wrong thing when determining deleted functions/methods.

* Scripts/prepare-ChangeLog:
(originalFile):
Use HEAD instead of origin/master in default Git case.

(generateFunctionLists):
Ensure that the end line is not less than the start line.

(extractLineRangeAfterChange):
Set the end line to the start line if the end line is less than the start line.

(extractLineRangeBeforeChange):
Ditto.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsScriptsprepareChangeLog">trunk/Tools/Scripts/prepare-ChangeLog</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (183029 => 183030)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog        2015-04-20 22:05:50 UTC (rev 183029)
+++ trunk/Tools/ChangeLog        2015-04-20 22:05:52 UTC (rev 183030)
</span><span class="lines">@@ -1,5 +1,47 @@
</span><span class="cx"> 2015-04-17  Matthew Hanson  &lt;matthew_hanson@apple.com&gt;
</span><span class="cx"> 
</span><ins>+        prepare-ChangeLog should ignore the preceeding function when processing the removal of a function.
+        https://bugs.webkit.org/show_bug.cgi?id=143897
+
+        Reviewed by David Kilzer.
+
+        This is a speculative fix that addresses two issues:
+
+        1. An off-by-one error which allowed ending lines to be less than starting lines when a hunk was a pure delete.
+        We were determining ending lines from combined diffs using the logic: End = Start + Offset - 1.
+
+        So for a hunk like &quot;@@ -723,10 +721,0 @@ bool foobar&quot;, we were generating the following starting/ending line pairs:
+        Before: (723, 729)
+        After: (721, 720)
+
+        Before is correct, but After should be (721, 721), since it represents the beginning and ending lines for the hunk.
+        Whether there are zero lines or one line in the hunk, the starting and ending line are the same.
+
+        This error was causing bad behavior on purely additive and purely subtractive hunks, but since we only refer
+        to After when generating ChangeLog output, the extractLineRangeBeforeChange had no visible effect on program output.
+
+        The fix is to set End to the max of Start + Offset - 1 and Start, rather than always using the former.
+
+        2. Creating git diffs from HEAD and not origin/master by default.
+
+        Hard-coding origin/master into the originalFile command has the disadvantage of causing the diff to fail entirely
+        when origin/master does not exist, and to do the wrong thing when determining deleted functions/methods.
+
+        * Scripts/prepare-ChangeLog:
+        (originalFile):
+        Use HEAD instead of origin/master in default Git case.
+
+        (generateFunctionLists):
+        Ensure that the end line is not less than the start line.
+
+        (extractLineRangeAfterChange):
+        Set the end line to the start line if the end line is less than the start line.
+
+        (extractLineRangeBeforeChange):
+        Ditto.
+
+2015-04-17  Matthew Hanson  &lt;matthew_hanson@apple.com&gt;
+
</ins><span class="cx">         Suppress warning in prepare-ChangeLog.
</span><span class="cx">         https://bugs.webkit.org/show_bug.cgi?id=143882
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkToolsScriptsprepareChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/Scripts/prepare-ChangeLog (183029 => 183030)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/Scripts/prepare-ChangeLog        2015-04-20 22:05:50 UTC (rev 183029)
+++ trunk/Tools/Scripts/prepare-ChangeLog        2015-04-20 22:05:52 UTC (rev 183030)
</span><span class="lines">@@ -60,7 +60,7 @@
</span><span class="cx"> use FindBin;
</span><span class="cx"> use Getopt::Long;
</span><span class="cx"> use lib $FindBin::Bin;
</span><del>-use List::Util qw/any/;
</del><ins>+use List::Util qw/any max/;
</ins><span class="cx"> use POSIX qw(strftime);
</span><span class="cx"> use VCSUtils;
</span><span class="cx"> 
</span><span class="lines">@@ -257,7 +257,7 @@
</span><span class="cx">         if ($mergeBase) {
</span><span class="cx">             $command .= &quot;$mergeBase&quot;;
</span><span class="cx">         } else {
</span><del>-            $command .= &quot;origin/master&quot;;
</del><ins>+            $command .= &quot;HEAD&quot;;
</ins><span class="cx">         }
</span><span class="cx">         $command .= &quot;:$file&quot;;
</span><span class="cx">     }
</span><span class="lines">@@ -287,7 +287,7 @@
</span><span class="cx">                     print STDERR &quot;WARNING: file $file contains the string DO_NOT_COMMIT, line $.\n&quot;;
</span><span class="cx">                 }
</span><span class="cx">                 my ($after_start, $after_end) = extractLineRangeAfterChange($_);
</span><del>-                if ($after_start &gt;= 0 &amp;&amp; $after_end &gt;= 0) {
</del><ins>+                if ($after_start &gt;= 0 &amp;&amp; $after_end &gt;= 0 &amp;&amp; $after_end &gt;= $after_start) {
</ins><span class="cx">                     push @{$line_ranges_after_changed{$file}}, [ $after_start, $after_end ];
</span><span class="cx">                 } elsif (/DO_NOT_COMMIT/) {
</span><span class="cx">                     print STDERR &quot;WARNING: file $file contains the string DO_NOT_COMMIT, line $.\n&quot;;
</span><span class="lines">@@ -2108,7 +2108,7 @@
</span><span class="cx">         $end = $4 || $2;
</span><span class="cx">     } elsif (isGit() &amp;&amp; $string =~ /^@@ -\d+(,\d+)? \+(\d+)(,(\d+))? @@/) {
</span><span class="cx">         $start = $2;
</span><del>-        $end = defined($4) ? $4 + $2 - 1 : $2;
</del><ins>+        $end = defined($4) ? max($start + $4 - 1, $start) : $start;
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     return ($start, $end);
</span><span class="lines">@@ -2125,7 +2125,7 @@
</span><span class="cx">         $end = $3 || $1 if $4 ne &quot;a&quot;;
</span><span class="cx">     } elsif (isGit() &amp;&amp; $string =~ /^@@ -(\d+)(,(\d+))? \+\d+(,\d+)? @@/) {
</span><span class="cx">         $start = $1;
</span><del>-        $end = defined($3) ? $3 + $1 - 1 : $1;
</del><ins>+        $end = defined($3) ? max($start + $3 - 1, $start) : $start;
</ins><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     return ($start, $end);
</span></span></pre>
</div>
</div>

</body>
</html>