Messages in this thread | | | Date | Tue, 20 Nov 2012 16:36:55 +0000 | From | Andy Whitcroft <> | Subject | Re: [PATCH v2] checkpatch: add double empty line check |
| |
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
| |