Message ID | 1353427570.6559.21.camel@lb-tlvb-eilong.il.broadcom.com |
---|---|
State | Not Applicable, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote: > I'm only testing the nextline if the current line is newly added. If I > got it right, when a line is newly added, the next line can be: > a. another new line > b. existing line (provided for context) > c. Does not exist since this is the end of the file (I missed this one > originally) > > It cannot just jump to the next hunk and it cannot be a deleted line, > right? Mostly that would be true. If the hunk is the last hunk and adds lines at the bottom of a file _and_ the context around it has blank lines then something. I think that would trip up this algorithm, reporting beyond the end of the hunk perhaps. -apw -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2012-11-20 at 16:14 +0000, Andy Whitcroft wrote: > On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote: > > I'm only testing the nextline if the current line is newly added. If I > > got it right, when a line is newly added, the next line can be: > > a. another new line > > b. existing line (provided for context) > > c. Does not exist since this is the end of the file (I missed this one > > originally) > > > > It cannot just jump to the next hunk and it cannot be a deleted line, > > right? > > Mostly that would be true. If the hunk is the last hunk and adds lines > at the bottom of a file _and_ the context around it has blank lines then > something. I think that would trip up this algorithm, reporting beyond > the end of the hunk perhaps. I do not want to cause any perl warning, but adding a new segment that ends with a new empty line above an existing empty line is something that I want to catch - so checking the next line (even if it is not new) is desired. Do you have other suggestions on how to implement something like that? I'm not saying that my patch is safe - I already missed a corner case when adding a line at the end of the file, but I'm willing to run more tests and see if I hit some perl warning. So how about running it on the last X changes in the kernel git tree? How many tests are enough to get reasonable confidant level? Thanks, Eilon -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 20, 2012 at 04:14:17PM +0000, Andy Whitcroft wrote: > On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote: > > I'm only testing the nextline if the current line is newly added. If I > > got it right, when a line is newly added, the next line can be: > > a. another new line > > b. existing line (provided for context) > > c. Does not exist since this is the end of the file (I missed this one > > originally) > > > > It cannot just jump to the next hunk and it cannot be a deleted line, > > right? Oh and in theory at least it could be a - line, though diff never generates things in that order. -apw -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 20, 2012 at 06:22:24PM +0200, Eilon Greenstein wrote: > On Tue, 2012-11-20 at 16:14 +0000, Andy Whitcroft wrote: > > On Tue, Nov 20, 2012 at 06:06:10PM +0200, Eilon Greenstein wrote: > > > I'm only testing the nextline if the current line is newly added. If I > > > got it right, when a line is newly added, the next line can be: > > > a. another new line > > > b. existing line (provided for context) > > > c. Does not exist since this is the end of the file (I missed this one > > > originally) > > > > > > It cannot just jump to the next hunk and it cannot be a deleted line, > > > right? > > > > Mostly that would be true. If the hunk is the last hunk and adds lines > > at the bottom of a file _and_ the context around it has blank lines then > > something. I think that would trip up this algorithm, reporting beyond > > the end of the hunk perhaps. > > I do not want to cause any perl warning, but adding a new segment that > ends with a new empty line above an existing empty line is something > that I want to catch - so checking the next line (even if it is not new) > is desired. Do you have other suggestions on how to implement something > like that? > > I'm not saying that my patch is safe - I already missed a corner case > when adding a line at the end of the file, but I'm willing to run more > tests and see if I hit some perl warning. So how about running it on the > last X changes in the kernel git tree? How many tests are enough to get > reasonable confidant level? I have been testing the patches there with some fake files to try and catch these indeed. I did incldue my take on how to solve this in previous replies. -apw -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/test.c b/test.c index e3f1362..5ced040 100644 --- a/test.c +++ b/test.c @@ -1,4 +1,5 @@ + /* there is an empty line just above me (the first line in this file) * nothing * nothing @@ -12,17 +13,8 @@ */ /* There is an empty line just below me */ -/* nothing - * nothing - * nothing - * nothing - * nothing - * nothing - * nothing - * nothing - * nothing - * nothing - */ + +/* just added a new empty line after deleting a segment */ /* nothing * nothing * nothing @@ -36,3 +28,4 @@ */ /* The last line (right below me) is empty */ +