Re: [PATCH v4 03/10] trailer: teach iterator about non-trailer lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
>> On 05/05/2024 02:37, Linus Arver wrote:
>>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>>> On 02/05/2024 05:54, Linus Arver via GitGitGadget wrote:
>>>>> From: Linus Arver <linus@ucla.edu>
>>>>>
>>>>> The new "raw" member is important because anyone currently not using the
>>>>> iterator is using trailer_info's raw string array directly to access
>>>>> lines to check what the combined key + value looks like. If we didn't
>>>>> provide a "raw" member here, iterator users would have to re-construct
>>>>> the unparsed line by concatenating the key and value back together again
>>>>> --- which places an undue burden for iterator users.
>>>>
>>>> Comparing the raw line is error prone as it ignores custom separators
>>>> and variations in the amount of space between the key and the value.
>>>> Therefore I'd argue that the sequencer should in fact be comparing the
>>>> trailer key and value separately rather than comparing the whole line.
>>>
>>> I agree, but that is likely beyond the scope of this series as the
>>> behavior of comparing the whole line was preserved (not introduced) by
>>> this series.
>>
>> Right but this series is changing the trailer iterator api to
>> accommodate the sub-optimal sequencer code. My thought was that if the
>> sequencer did the right thing we wouldn't need to expose the raw line in
>> the iterator in the first place.
> 
> Well, having familiarized myself with the trailer machinery I was more
> comfortable updating this area than reworking the details of the
> sequencer code.
> 
> To me the sequencer code is a bit hard to read so I feel more
> comfortable updating the trailer code as I did here in this series. I
> also have another 40, 50 patches in the trailer area I want to continue
> pushing up for review, so I would rather focus on that first before
> coming back to the sequencer to try to clean it up. My other trailer
> patches make the trailer API more precise and aware of the separator and
> spaces around it,

> so using those richer interfaces later would make it
> easier to clean up the sequencer area, I think.
> 
> So in summary I would rather not get into refactoring the sequencer at
> this time.