git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
DSDragan Simic <dsimic@manjaro.org>
Date
Mar 18, 2024, 21:57 UTC
Message-ID
<f37d753485094a3ba66fde5e85d0e2dc@manjaro.org>
In-Reply-To
<CAPig+cQgejS5dFpOYkMT49kwn5zvn5ihvyFqViFwHhEmtr9mqw@mail.gmail.com>
On 2024-03-18 20:21, Eric Sunshine wrote:
Show 40 quoted lines
> 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.
Totally agreed, let's keep this plan for the follow-up patches.
Previous: Eric SunshineNext: Dragan Simic
Message 15 of 20 in “Fix a bug in configuration parsing, and improve tests and documentation”
  1. 0/5 Fix a bug in configuration parsing, and improve tests and documentationDragan Simic, Mar 17, 2024
  2. 1/5 config: minor addition of whitespaceDragan Simic, Mar 17, 2024
  3. 2/5 config: really keep value-internal whitespace verbatimDragan Simic, Mar 17, 2024
  4. 3/5 test: introduce new x_to_tab() helper functionDragan Simic, Mar 17, 2024
  5. Eric SunshineMar 17, 2024
  6. Dragan SimicMar 17, 2024
  7. 4/5 t1300: add more tests for whitespace and inline commentsDragan Simic, Mar 17, 2024
  8. Eric SunshineMar 17, 2024
  9. Eric SunshineMar 17, 2024
  10. Dragan SimicMar 17, 2024
  11. Eric SunshineMar 18, 2024
  12. Junio C HamanoMar 18, 2024
  13. Dragan SimicMar 18, 2024
  14. Eric SunshineMar 18, 2024
  15. Dragan SimicMar 18, 2024
  16. Dragan SimicMar 18, 2024
  17. Eric SunshineMar 18, 2024
  18. Junio C HamanoMar 18, 2024
  19. Dragan SimicMar 18, 2024
  20. 5/5 config.txt: describe handling of whitespace furtherDragan Simic, Mar 17, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.