Re: [PATCH] git-p4 submit: prevent 'Jobs' section from being removed from p4 change log

[Michael Horowitz <michael.horowitz@ieee.org>]

On Sat, Feb 26, 2011 at 2:37 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Michael Horowitz <michael.horowitz@ieee.org> writes:
>
>> In an attempt to overwrite the 'Description:' section of the p4 change
>> log to include the git commit messages, it also overwrote the 'Jobs:'
>> section.  This fix restores the 'Job:' section.
>>
>> Signed-off-by: Michael Horowitz <michael.horowitz@ieee.org>
>> ---
>>  contrib/fast-import/git-p4 |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/contrib/fast-import/git-p4 b/contrib/fast-import/git-p4
>> index a92beb6..8b00fd8 100755
>> --- a/contrib/fast-import/git-p4
>> +++ b/contrib/fast-import/git-p4
>> @@ -570,7 +570,7 @@ class P4Submit(Command):
>>                 continue
>>
>>             if inDescriptionSection:
>> -                if line.startswith("Files:"):
>> +                if line.startswith("Files:") or line.startswith("Jobs:"):
>>                     inDescriptionSection = False
>>                 else:
>>                     continue
>
> This is not a new issue with the code, but it makes me wonder if the
> output you are reading from guaranteed to have these lines in the same
> order.  Otherwise the next bug report and/or patch would add another
> similar looking line.startswith("SomethingElse:") to this statement, and
> we wouldn't know when to stop, would we?
>
> Will queue anyway, though.  Thanks.
>
>

Yes, you are correct, it could be written in a more robust way.
Ideally, with a proper spec, the parser can be written to handle all
the cases.  Unfortunately, I am not familiar enough to do much more
than fix the immediate issue I am having.  I only know enough Python
to make this simple change, and it seems to work in my tests.

Thanks,

Mike