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

On Mon, Mar 18, 2024 at 4:37 AM Dragan Simic [off-list ref] wrote:

On 2024-03-18 05:38, Junio C Hamano wrote:

quoted

      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.