On 2024-03-18 20:21, Eric Sunshine wrote:
On Mon, Mar 18, 2024 at 4:37 AM Dragan Simic <dsimic@xxxxxxxxxxx>
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.
Totally agreed, let's keep this plan for the follow-up patches.
