Re: [RFC PATCH 3/4] range-diff: add section header instead of diff header

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

 



On Sun, Apr 14, 2019 at 5:10 PM Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> [...]
> Introduce a new range diff hunk header, that's enclosed by "##",
> similar to how line numbers in diff hunks are enclosed by "@@", and
> give human readable information of what exactly happened to the file,
> including the file name.
> [...]
> Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
> ---
> diff --git a/range-diff.c b/range-diff.c
> @@ -90,8 +91,37 @@ static int read_patches(const char *range, struct string_list *list)
> +               } else if (starts_with(line.buf, "--- ")) {
> +                       if (!strcmp(line.buf, "--- /dev/null"))
> +                               strbuf_remove(&line, 0, 4);
> +                       else
> +                               strbuf_remove(&line, 0, 6);
> +                       strbuf_rtrim(&line);
> +                       strbuf_reset(&filename_a);
> +                       strbuf_addbuf(&filename_a, &line);
> +               } else if (starts_with(line.buf, "+++ ")) {

At this point, we know that line.buf starts with "+++"...

> +                       strbuf_addstr(&buf, " ## ");
> +                       if (!strcmp(line.buf, "--- /dev/null"))

so, it seems unlikely that it's ever going to match "--- /dev/null".

> +                               strbuf_remove(&line, 0, 4);
> +                       if (!strcmp(filename_a.buf, "/dev/null")) {
> +                               strbuf_addstr(&buf, "new file ");
> +                               strbuf_addbuf(&buf, &line);
> +                       } else if (!strcmp(line.buf, "/dev/null")) {
> +                               strbuf_addstr(&buf, "removed file ");
> +                               strbuf_addbuf(&buf, &line);
> +                       } else if (strbuf_cmp(&filename_a, &line)) {
> +                               strbuf_addstr(&buf, "renamed file ");
> +                               strbuf_addbuf(&buf, &filename_a);
> +                               strbuf_addstr(&buf, " -> ");
> +                               strbuf_addbuf(&buf, &line);
> +                       } else {
> +                               strbuf_addstr(&buf, "modified file ");
> +                               strbuf_addbuf(&buf, &line);
> +                       }

All of these disposition strings end with "file", which seems
redundant. Short and sweet "new", "removed", "renamed", "modified"
provide just as much useful information.

Also, should these strings be localizable? Alternately, rather than
using prose to describe the disposition, perhaps do so symbolically
(thus universally), say with "+", "-", "->", "*" (or ""),
respectively?



[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