Re: [PATCH 02/19] diff: move line ending check into emit_hunk_header
- From
Stefan Beller <sbeller@google.com>
- Date
- May 15, 2017, 16:13 UTC
- Message-ID
- <CAGZ79kYLNojFHE7mhgmy84-tRbNN66Kt=DFQoMAKz_p4=_m8fA@mail.gmail.com>
- In-Reply-To
- <xmqqk25ivcnb.fsf@gitster.mtv.corp.google.com>
On Sun, May 14, 2017 at 11:48 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 36 quoted lines
> Stefan Beller <sbeller@google.com> writes: > >> In a later patch, I want to propose an option to detect&color >> moved lines in a diff, which cannot be done in a one-pass over >> the diff. Instead we need to go over the whole diff twice, >> because we cannot detect the first line of the two corresponding >> lines (+ and -) that got moved. >> >> So to prepare the diff machinery for two pass algorithms >> (i.e. buffer it all up and then operate on the result), >> move all emissions to places, such that the only emitting >> function is emit_line_0. >> >> This patch moves code that is conceptually part of >> emit_hunk_header, but was using output in fn_out_consume, >> back to emit_hunk_header. > > Makes sort-of sense. If I were selling this patch, I'd remove the > first two paragraph and stress on how completing the line inside > emit_hunk_header() is conceptually cleaner than doing it outside. > > emit_hunk_header() function is responsible for assembling a > hunk header and calling emit_line() to send the hunk header > to the output file. Its only caller fn_out_consume() needs > to prepare for a case where the function emits an incomplete > line and add the terminating LF. > > Instead make sure emit_hunk_header() to always send a > completed line to emit_line(). > > or something like that. > > Note that I am not saying "buffering the entire diff in-core? why > should we support such a use case?". I am saying that this change > is a clean-up that is justifiable, without having to answer such a > question.
Right, the first couple patches are more cleanup than preparation. I considered sending them on their own, but then decided to rather include it in this series.
I'll reword the commit message for a resend.
Thanks, Stefan