Re: [PATCH v2 4/5] t1300: add more tests for whitespace and inline comments

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Mar 18, 2024 at 4:37 AM Dragan Simic <dsimic@manjaro.org> wrote:
> On 2024-03-18 05:38, Junio C Hamano wrote:
> >       sed -e 's/^|//' -e 's/Q/   /g' >.git/config <<-\EOF
> >       |[section]
> >       |       solid = rock
> >       |       sparse = big QQ blue
> >       |       ...
> >       EOF
> >
> This looks quite neat.  Furthermore, I think we should also consider
> the already existing tests in the t1300 that contain such indentation.
> As I already explained in my earlier response to Eric, [1] the choice
> of including the indentation or not seems random to me, so we should
> perhaps consider taking some broader approach.
>
> How about this as a plan for moving forward:
>
> 1) Sprinkle a couple of tests onto the t1300, which try to be
>     focused on the verification of the indentation-handling logic;
>     maybe those additional tests could be even seen as redundant,
>     but I think they can only help with the test coverage
>
> 2) Create a new helper function that uses the logic you described
>     above, to make it simpler to include the indentation into configs
>
> 3) Finally, propagate the use of this new helper function into the
>     new test and the already existing tests in the t1300 that already
>     include the indentation
>
> I'd be happy to implement all of the above-proposed steps in the next
> couple of days.  Sure, it would be quite time-consuming, especially the
> third proposed step, but it should be worth it in the long run.

As noted in my other response, while such cleanups may be nice in the
long-run, the bug-fix patch series under discussion should not be held
hostage by an ever-increasing set of potential cleanups. Let's focus
on landing the current series; these tangential cleanups can be done
in a separate series.