Hi Linus
On 09/05/2024 08:11, Linus Arver wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
On 05/05/2024 02:37, Linus Arver wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
On 02/05/2024 05:54, Linus Arver via GitGitGadget wrote:
From: Linus Arver <linus@xxxxxxxx>
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,
I'm a bit confused by this as it looks like the
trailer_iterator_advance() already respects trailer.separators and
trims the key and value.
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.
Far enough, I hoped I might have time over the weekend to look at the
sequencer code in more detail but that didn't work out. So long as we're
not building bad abstractions into the trailer iterator to accommodate
the sequencer we can come back and clean up append_signoff() later.
Best Wishes
Phillip
