Re: [PATCH] git-add-interactive: edit current file in editor

[Eric Sunshine <sunshine@sunshineco.com>]

Aside from whether or not this change is desirable, see a few pointers
below to improve the patch...

On Monday, July 27, 2015, Sina Siadat <siadat@gmail.com> wrote:
> Adds a new option 'o' to the 'add -p' command that lets you open and edit the
> current file.

Imperative mood: "Add a new option..."

> The existing 'e' mode is used to manually edit the hunk.  The new 'o' option
> allows you to open and edit the file without having to quit the loop. The hunks
> are updated when the editing is done, and the user will be able to review the
> updated hunks.  Without this option you would have to quit the loop, edit the
> file, and execute 'add -p filename' again.

This descriptive material belongs in the commit message. Good.

> I would appreciate it if you could let me know what you think about this
> option. I will write more tests if there is any interest at all.

This, however, is commentary, which, while useful as part of the
submission process, does not belong in the commit message. Therefore,
it should be placed below the "---" line just before the diffstat.

> Thank you. :)

Missing sign-off. See Documentation/SubmittingPatches.

More below...

> ---
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index deae948..e5dd1c6 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -98,6 +98,12 @@ test_expect_success 'dummy edit works' '
>         test_cmp expected diff
>  '
>
> +test_expect_success 'dummy open works' '
> +       (echo o; echo a) | git add -p &&

Some alternatives, which may or may not read better, but at least
avoid a process creation or two:

    { echo o; echo a; } | git add -p &&

    printf "%s\n" o a | git add -p &&

    printf "o\na\n" | git add -p &&

Those are just suggestions; not necessarily a request for change.

> +       git diff > diff &&

Style: >diff

> +       test_cmp expected diff
> +'
> +
>  test_expect_success 'setup patch' '
>  cat >patch <<EOF
>  @@ -1,1 +1,4 @@
> --
> 2.5.0.rc3.2.g6f9504c.dirty