Re: [PATCH v2] range-diff: support reading mbox files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dscho

On 22/11/2022 07:40, Johannes Schindelin wrote:
[...]
I'm a bit confused by this sed command, I've annotated it with my probably
flawed understanding.

       ++	sed -e "/^From: .*/{
       ++		h

This stores the From: header in the hold space

👍

       ++		s/.*/From: Bugs Bunny <bugs@xxxxxx>/

This changes the From: header in the pattern space

👍

       ++		:1
       ++		N
       ++		/[ -z]$/b1

We loop until we find a line that does not end with a space, letter or number
adding the lines to the hold space

I would have _loved_ to match on an empty line, i.e. `/[^\n]$/b1`. But
that construct is not understood by the `sed` on macOS.

I don't think it matters much but can't you match an empty line with /^$/ ? Then you can loop on non-empty lines with /^$/!b1

I even went so far as to search for the source code of a BSD `sed` (and I
found it, and modified the code so that it builds on Linux, see
https://github.com/dscho/sed-bsd) to try a couple of things, but could not
make it work with any variation of `\n`. Therefore, I settled on expecting
all the lines in the commit header to end in printable ASCII characters
between the space and the `z`.

       ++		G

This appends the hold space to the pattern space, then the pattern space is
printed.

👍

Doesn't this mean we end up with two From: headers? Is the in-body From:
line already present?

There is no in-body `From:` header because the patch author matches the
`GIT_AUTHOR_IDENT` that is in effect while running the `format-patch`
command.

Let me show you what this `sed` call deals with. In the local test run, it
modified an `mbox` starting with this:

Thanks for taking the trouble to show the mbox files, I didn't have time to run the tests my self yesterday. The processed mbox file looks good.

	From 4d39cb329d3ef4c8e69b43859c2e11adb83f8613 Mon Sep 17 00:00:00 2001
	From: Thomas Rast <trast@xxxxxxxxxxx>
	Date: Mon, 22 Jul 2013 11:23:44 +0200
	Subject: [PATCH 1/3] s/4/A/ + add other-file

	---
	  file       | 2 +-
	  other-file | 0
	  [...]

to a modified `mbox` that starts with this:

	From 4d39cb329d3ef4c8e69b43859c2e11adb83f8613 Mon Sep 17 00:00:00 2001
	From: Bugs Bunny <bugs@xxxxxx>
	Date: Mon, 22 Jul 2013 11:23:44 +0200
	Subject: [PATCH 1/3] s/4/A/ + add other-file

	From: Thomas Rast <trast@xxxxxxxxxxx>
	---
	  file       | 2 +-
	  other-file | 0
	  [...]


       ++	}" <mbox >mbox.from &&
       ++	git range-diff mode-only-change..topic mbox:./mbox.from
       >actual.from &&
       ++	test_cmp expect actual.from &&
       ++
       ++	append_cr <mbox >mbox.cr &&
       ++	test_must_fail git range-diff \
       ++		mode-only-change..topic mbox:./mbox.cr 2>err &&
       ++	grep CR/LF err &&

Thanks for adding that

Thank you for the suggestion to add it!

And thank you again for modeling how to perform actionable, helpful and
productive code reviews,

Thanks for your kind words, I glad you found my comments helpful

Best Wishes

Phillip



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux