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

Re: [PATCH v2 3/4] rebase: fix garbled progress display with '-x'

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jun 25, 2019, 18:00 UTC
Message-ID
<573b9280-939b-879e-6e5e-b63779836740@gmail.com>
In-Reply-To
<20190625113103.GB10853@szeder.dev>

Hi Gábor and Dscho On 25/06/2019 12:31, SZEDER Gábor wrote:

Show 68 quoted lines
> On Tue, Jun 25, 2019 at 11:08:04AM +0100, Phillip Wood wrote:
>>>> Rebasing (1/4)QRebasing (2/4)QRebasing (3/4)QRebasing (4/4)QQ                                                                                QSuccessfully rebased and updated refs/heads/missing-commit.
> 
> 
>>>> May I please *strongly* suggest to fix this first? It should
>>>>
>>>> - completely lose that last line,
>>>> - be inserted into the test case itself so that e.g. disk full problems are
>>>>   caught and logged properly, and
>>>> - the `test_i18ncmp expect actual` call in the test case should be replaced
>>>>   by something like:
>>>>
>>>> 	sed "\$d" <actual >actual-skip-progress &&
>>>> 	test_i18ncmp expect actual-skip-progress
>>>>
>>>> This should obviously be made as a separate, introductory patch (probably with
>>>> a less scathing commit message than my comments above would suggest).
>>>>
>>>> And that would also remove the double-yuck.
>>>
>>> Unfortunately, this addresses only one of the "Yuck"s; see v3 of this
>>> patch series [1].
>>>
>>> The other yucks affect the following four tests in
>>> 't3420-rebase-autostash.sh':
>>>
>>>   16 - rebase --merge --autostash: check output
>>>   23 - rebase --merge: check output with conflicting stash
>>>   26 - rebase --interactive --autostash: check output
>>>   33 - rebase --interactive: check output with conflicting stash
>>>
>>> These tests come from commits b76aeae553 (rebase: add regression tests
>>> for console output, 2017-06-19) and 7d70e6b902 (rebase: add more
>>> regression tests for console output, 2017-06-19), and are specifically
>>> about checking the (whole) console output of 'git rebase', so I left
>>> the updates to them as they were.
>>>
>>> In any case, Cc-ing Phillip to discuss whether something could be done
>>> about them (now perhaps preferably (for me :) as a follow-up, and not
>>> another preparatory patches).
>>
>> Those tests were added to check that `git stash` was being silenced (see
>> 79a6226981 ("rebase -i: silence stash apply", 2017-05-18)).
> 
> Hmm, so would it then be possible/sensible to do something like this
> instead:
> 
>   git rebase --options... >out &&
>   test_i18ngrep ! "<something specific that only 'git stash' would print if it wasn't silenced>" out
> 
>> I can have a
>> think about a better way to do that, but is it still a problem? I just
>> tried to take a look at your CI output and
>> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=11406
>> seems to be all green - have I missed something or has Gábor fixed the
>> issue?
> 
> No, it was Dscho who fixed the Azure CI issue, see
> 
>   https://public-inbox.org/git/nycvar.QRO.7.76.6.1906141356140.44@tvgsbejvaqbjf.bet/
> 
> elsewhere in this thread (it's a long one, but well worth the read
> IMO).
> 
> However, the point here is not that Azure CI failure, but rather the
> fact that tests had to be updated to include the new line clearing
> sequence in the expected output, and that "Q<...80 spaces...>Q" looks
> yuck indeed.

I've come up with a sed based solution to remove the progress notifications from the output - see https://github.com/gitgitgadget/git/pull/276 If you're both happy with the basic idea I'll clean it up and submit it (I've just noticed I left some debugging bits in one of the tests). I was worried using "\r" in a sed expression wouldn't be portable but the tests pass on gitgitgadget. If it proves to be a problem on other platforms we could use always tr to convert "\r" to some unique string and use that string in the sed expression.

Best Wishes
Phillip
Previous: Phillip WoodNext: Johannes Schindelin
Message 23 of 41 in “rebase: fix garbled progress display with '-x'”
  1. rebase: fix garbled progress display with '-x'SZEDER Gábor, Apr 30, 2019
  2. Johannes SchindelinApr 30, 2019
  3. SZEDER GáborMay 1, 2019
  4. Johannes SchindelinMay 3, 2019
  5. SZEDER GáborJun 11, 2019
  6. 0/4 rebase/progress: add and use term_clear_line()SZEDER Gábor, Jun 11, 2019
  7. 2/4 pager: add a helper function to clear the last line in the terminalSZEDER Gábor, Jun 11, 2019
  8. 1/4 t3404-rebase-interactive: use the 'q_to_cr' helperSZEDER Gábor, Jun 11, 2019
  9. 3/4 rebase: fix garbled progress display with '-x'SZEDER Gábor, Jun 11, 2019
  10. Junio C HamanoJun 11, 2019
  11. SZEDER GáborJun 11, 2019
  12. Johannes SchindelinJun 12, 2019
  13. SZEDER GáborJun 12, 2019
  14. Johannes SchindelinJun 13, 2019
  15. Johannes SchindelinJun 14, 2019
  16. SZEDER GáborJun 17, 2019
  17. SZEDER GáborJun 17, 2019
  18. SZEDER GáborJun 17, 2019
  19. SZEDER GáborJun 24, 2019
  20. Phillip WoodJun 25, 2019
  21. SZEDER GáborJun 25, 2019
  22. Phillip WoodJun 25, 2019
  23. Phillip WoodJun 25, 2019
  24. Johannes SchindelinJun 25, 2019
  25. Phillip WoodJun 25, 2019
  26. Junio C HamanoJun 11, 2019
  27. SZEDER GáborJun 11, 2019
  28. Junio C HamanoJun 12, 2019
  29. 4/4 progress: use term_clear_line()SZEDER Gábor, Jun 11, 2019
  30. Johannes SchindelinJun 12, 2019
  31. 0/5 rebase/progress: add and use term_clear_line()SZEDER Gábor, Jun 24, 2019
  32. 2/5 t3404: make the 'rebase.missingCommitsCheck=ignore' test more focusedSZEDER Gábor, Jun 24, 2019
  33. Johannes SchindelinJun 25, 2019
  34. 1/5 t3404: modernize here doc styleSZEDER Gábor, Jun 24, 2019
  35. Johannes SchindelinJun 25, 2019
  36. 3/5 pager: add a helper function to clear the last line in the terminalSZEDER Gábor, Jun 24, 2019
  37. 4/5 rebase: fix garbled progress display with '-x'SZEDER Gábor, Jun 24, 2019
  38. 4/5 rebase: fix garbled progress display with '-x'SZEDER Gábor, Jun 27, 2019
  39. 5/5 progress: use term_clear_line()SZEDER Gábor, Jun 24, 2019
  40. Johannes SchindelinJun 25, 2019
  41. Junio C HamanoJun 25, 2019

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.