Re: [PATCH] sequencer: require trailing NL in footers

2017-04-26 Thread Brian Norris
On Tue, Apr 25, 2017 at 03:30:56PM -0700, Brian Norris wrote: > On Tue, Apr 25, 2017 at 02:56:53PM -0700, Stefan Beller wrote: > > In that case: Is it needed to hint at how this bug occurred in the wild? > > (A different Git implementation, which may be fixed now?) > > I might contact the original

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Nieder
Johannes Schindelin wrote: >> On Tue, Apr 25, 2017 at 2:51 PM, Jonathan Tan >> wrote: >>> On 04/25/2017 02:04 PM, Stefan Beller wrote: Do we want to test for this use case in the future? [...] >>> I'm not sure of the value of including a test for this specific use case, >>> because Git norm

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Nieder
Hi, Jonathan Tan wrote: > In commit 967dfd4 ("sequencer: use trailer's trailer layout", > 2016-11-29), sequencer was taught to use the same mechanism as > interpret-trailers to determine the nature of the trailer of a commit > message (referred to as the "footer" in sequencer.c). However, the > r

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Brian Norris
On Tue, Apr 25, 2017 at 02:56:53PM -0700, Stefan Beller wrote: > In that case: Is it needed to hint at how this bug occurred in the wild? > (A different Git implementation, which may be fixed now?) I might contact the original author, but it's easy enough to imagine automated 'filter-branch' scrip

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Johannes Schindelin
Hi Stefan, On Tue, 25 Apr 2017, Stefan Beller wrote: > On Tue, Apr 25, 2017 at 2:51 PM, Jonathan Tan > wrote: > > On 04/25/2017 02:04 PM, Stefan Beller wrote: > >> > >> Thanks for the fix. :) > >> Do we want to test for this use case in the future? > > > > > > Thanks! > > > > I'm not sure of th

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Stefan Beller
On Tue, Apr 25, 2017 at 2:51 PM, Jonathan Tan wrote: > On 04/25/2017 02:04 PM, Stefan Beller wrote: >> >> Thanks for the fix. :) >> Do we want to test for this use case in the future? > > > Thanks! > > I'm not sure of the value of including a test for this specific use case, > because Git normally

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Johannes Schindelin
Hi Jonathan, On Tue, 25 Apr 2017, Jonathan Tan wrote: > Thanks for the bug report. Here's a fix - I've verified this with the > way to reproduce provided in the original e-mail, and it seems to work > now. If there is a straight-forward way to convert the manual test into an automated one, it wo

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Tan
On 04/25/2017 02:04 PM, Stefan Beller wrote: Thanks for the fix. :) Do we want to test for this use case in the future? Thanks! I'm not sure of the value of including a test for this specific use case, because Git normally does not create commit messages with no trailing newlines. (To test t

Re: [PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Stefan Beller
On Tue, Apr 25, 2017 at 12:06 PM, Jonathan Tan wrote: > In commit 967dfd4 ("sequencer: use trailer's trailer layout", > 2016-11-29), sequencer was taught to use the same mechanism as > interpret-trailers to determine the nature of the trailer of a commit > message (referred to as the "footer" in s

[PATCH] sequencer: require trailing NL in footers

2017-04-25 Thread Jonathan Tan
In commit 967dfd4 ("sequencer: use trailer's trailer layout", 2016-11-29), sequencer was taught to use the same mechanism as interpret-trailers to determine the nature of the trailer of a commit message (referred to as the "footer" in sequencer.c). However, the requirement that a footer end in a ne