[Outreachy][RFC/PATCH] notes: teach the -e option to edit messages in editor #1817
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, "brian m. carlson" wrote (reply to this): On 2024-10-19 at 00:14:13, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle <[email protected]>
>
> Notes can be added to a commit using the -m (message),
> -C (copy a note from a blob object) or
> -F (read the note from a file) options.
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
>
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and editted
> after the messages have been provided through -[mF].
I don't use the notes feature, but I definitely see how this is valuable
there much as it is for `git commit`.
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 8c26e455269..02cdfdf1c9d 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> parse_reedit_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_BOOL(0, "allow-empty", &allow_empty,
> N_("allow storing empty note")),
> OPT_CALLBACK_F(0, "separator", &separator,
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 99137fb2357..7f45a324faa 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -1567,4 +1567,33 @@ test_expect_success 'empty notes do not invoke the editor' '
> git notes remove HEAD
> '
>
> +test_expect_success '"git notes add" with -m/-F invokes the editor with -e' '
> + test_commit 19th &&
> + GIT_EDITOR="true" git notes add -m "note message" -e &&
> + git notes remove HEAD &&
> + echo "message from file" >file_1 &&
> + GIT_EDITOR="true" git notes add -F file_1 -e &&
> + git notes remove HEAD
> +'
Maybe I don't understand what this is supposed to be testing (and if so,
please correct me), but how are we verifying that the editor is being
invoked? If we're invoking `true`, then that doesn't change the message
in any way, so if we suddenly stopped invoking the editor, I don't think
this would fail.
Maybe we could use something else as `GIT_EDITOR` instead. For example,
if we did `GIT_EDITOR="perl -pi -e s/file/editor/" git notes add -F file_1 -e`
(with an appropriate PERL prerequisite), then we could test that the
message after the fact was "message from editor", which would help us
verify that both the `-F` and `-e` options were being honoured.
(Similar things can be said about the tests you added below this as
well.)
I suggest Perl here because `sed -i` is nonstandard and not portable,
but you could also set up a fake editor script as in t7004, which would
avoid the need for the Perl dependency by using `sed` with a temporary
file. That might be more palatable to the project at large, but I'd be
fine with either approach.
Do you think there are any cases where testing the `--no-edit`
functionality might be helpful? For example, is `git notes edit` ever
useful to invoke with such an option, like one might do with `git commit
amend`? (This isn't rhetorical, since the notes code is one of the areas
of Git I'm least familiar with, so I ask both because I'm curious and if
you think it's a useful thing to do, then tests might be a good idea.)
--
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA |
|
User |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): Hi Samuel
On Sat, Oct 19, 2024, at 02:14, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle <[email protected]>
>
> Notes can be added to a commit using the -m (message),
> -C (copy a note from a blob object) or
> -F (read the note from a file) options.
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
Thanks for this work. I think part of the motivation here is to make
git-notes(1) act more in line with the conventions from git-commit(1),
which is always nice to see.
It’s also useful in its own right.
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and editted
> after the messages have been provided through -[mF].
Here you explain how the end-user will benefit from this change. Nice.
It’s important to explain the background, what is being done, and why it
is being done. And this commit message does all of that. |
|
User |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): Hi brian and Samuel
On Sat, Oct 19, 2024, at 02:38, brian m. carlson wrote:
> On 2024-10-19 at 00:14:13, Samuel Adekunle Abraham via GitGitGadget wrote:
>> +test_expect_success '"git notes add" with -m/-F invokes the editor with -e' '
>> + test_commit 19th &&
>> + GIT_EDITOR="true" git notes add -m "note message" -e &&
>> + git notes remove HEAD &&
>> + echo "message from file" >file_1 &&
>> + GIT_EDITOR="true" git notes add -F file_1 -e &&
>> + git notes remove HEAD
>> +'
>
> Maybe I don't understand what this is supposed to be testing (and if so,
> please correct me), but how are we verifying that the editor is being
> invoked? If we're invoking `true`, then that doesn't change the message
> in any way, so if we suddenly stopped invoking the editor, I don't think
> this would fail.
I also didn’t understand these tests.
There is this test in this file/test suite which tests the negative
case:
test_expect_success 'empty notes do not invoke the editor' '
test_commit 18th &&
GIT_EDITOR="false" git notes add -C "$empty_blob" --allow-empty &&
git notes remove HEAD &&
GIT_EDITOR="false" git notes add -m "" --allow-empty &&
git notes remove HEAD &&
GIT_EDITOR="false" git notes add -F /dev/null --allow-empty &&
git notes remove HEAD
'
And this works because the commands would fail if the editor was invoked:
error: there was a problem with the editor 'false'
But this doesn’t work for `true`.
> Maybe we could use something else as `GIT_EDITOR` instead. For example,
> if we did `GIT_EDITOR="perl -pi -e s/file/editor/" git notes add -F file_1 -e`
> (with an appropriate PERL prerequisite), then we could test that the
> message after the fact was "message from editor", which would help us
> verify that both the `-F` and `-e` options were being honoured.
> (Similar things can be said about the tests you added below this as
> well.)
This file defines a `fake_editor`:[1]
write_script fake_editor <<\EOF
echo "$MSG" >"$1"
echo "$MSG" >&2
EOF
GIT_EDITOR=./fake_editor
export GIT_EDITOR
And it looks like this is how it is used:
test_expect_success 'create notes' '
MSG=b4 git notes add &&
test_path_is_missing .git/NOTES_EDITMSG &&
git ls-tree -r refs/notes/commits >actual &&
test_line_count = 1 actual &&
echo b4 >expect &&
git notes show >actual &&
test_cmp expect actual &&
git show HEAD^ &&
test_must_fail git notes show HEAD^
'
So it seems that the new tests here should use the `test_cmp expect
actual` style.
† 1: The different test files use both `fake_editor`, `fake-editor`,
and `fakeeditor`.
> Do you think there are any cases where testing the `--no-edit`
> functionality might be helpful? For example, is `git notes edit` ever
> useful to invoke with such an option, like one might do with `git commit
> amend`? (This isn't rhetorical, since the notes code is one of the areas
> of Git I'm least familiar with, so I ask both because I'm curious and if
> you think it's a useful thing to do, then tests might be a good idea.)
Yes, that is useful (both as a use-case and as a regression test[2]).
git-notes(1) is often used to programmatically add metadata:
git show todo:post-applypatch | grep -C5 refs/notes/amlog
(And this non-interactive example is not affected by this change since
`-e` is required in order to invoke the editor)
† 2: I seem to recall a regression in how git-notes(1) chose to invoke
the editor or not |
|
On Sat, Oct 19, 2024 at 1:43 AM gitgitgadget[bot]
***@***.***> wrote:
On the Git mailing list, "brian m. carlson" wrote (reply to this):
On 2024-10-19 at 00:14:13, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle ***@***.***>
>
> Notes can be added to a commit using the -m (message),
> -C (copy a note from a blob object) or
> -F (read the note from a file) options.
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
>
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and editted
> after the messages have been provided through -[mF].
I don't use the notes feature, but I definitely see how this is valuable
there much as it is for `git commit`.
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 8c26e455269..02cdfdf1c9d 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> parse_reedit_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_BOOL(0, "allow-empty", &allow_empty,
> N_("allow storing empty note")),
> OPT_CALLBACK_F(0, "separator", &separator,
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 99137fb2357..7f45a324faa 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -1567,4 +1567,33 @@ test_expect_success 'empty notes do not invoke the editor' '
> git notes remove HEAD
> '
>
> +test_expect_success '"git notes add" with -m/-F invokes the editor with -e' '
> + test_commit 19th &&
> + GIT_EDITOR="true" git notes add -m "note message" -e &&
> + git notes remove HEAD &&
> + echo "message from file" >file_1 &&
> + GIT_EDITOR="true" git notes add -F file_1 -e &&
> + git notes remove HEAD
> +'
Maybe I don't understand what this is supposed to be testing (and if so,
please correct me), but how are we verifying that the editor is being
invoked? If we're invoking `true`, then that doesn't change the message
in any way, so if we suddenly stopped invoking the editor, I don't think
this would fail.
Hello Brian,
Thanks for your review and feedback.
Yes, I realized the tests do not show a way to determine that the
editor changed the message.
Thank you your observation and pointing out to me.
Maybe we could use something else as `GIT_EDITOR` instead. For example,
if we did `GIT_EDITOR="perl -pi -e s/file/editor/" git notes add -F file_1 -e`
(with an appropriate PERL prerequisite), then we could test that the
message after the fact was "message from editor", which would help us
verify that both the `-F` and `-e` options were being honoured.
(Similar things can be said about the tests you added below this as
well.)
Okay this is duly noted.
I suggest Perl here because `sed -i` is nonstandard and not portable,
but you could also set up a fake editor script as in t7004, which would
avoid the need for the Perl dependency by using `sed` with a temporary
file. That might be more palatable to the project at large, but I'd be
fine with either approach.
I will look into this and also take a close look at the functions
which define the implementation
for the GIT_EDITOR which are the write_script with fake_editor as file.
… Do you think there are any cases where testing the `--no-edit`
functionality might be helpful? For example, is `git notes edit` ever
useful to invoke with such an option, like one might do with `git commit
amend`? (This isn't rhetorical, since the notes code is one of the areas
of Git I'm least familiar with, so I ask both because I'm curious and if
you think it's a useful thing to do, then tests might be a good idea.)
--
brian m. carlson (they/them or he/him)
Toronto, Ontario, CA
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Sat, Oct 19, 2024 at 11:28 AM Kristoffer Haugsbakk
<[email protected]> wrote:
>
> Hi Samuel
>
> On Sat, Oct 19, 2024, at 02:14, Samuel Adekunle Abraham via GitGitGadget wrote:
> > From: Abraham Samuel Adekunle <[email protected]>
> >
> > Notes can be added to a commit using the -m (message),
> > -C (copy a note from a blob object) or
> > -F (read the note from a file) options.
> > When these options are used, Git does not open an editor,
> > it simply takes the content provided via these options and
> > attaches it to the commit as a note.
>
> Thanks for this work. I think part of the motivation here is to make
> git-notes(1) act more in line with the conventions from git-commit(1),
> which is always nice to see.
>
> It’s also useful in its own right.
>
> > Improve flexibility to fine-tune the note before finalizing it
> > by allowing the messages to be prefilled in the editor and editted
> > after the messages have been provided through -[mF].
>
> Here you explain how the end-user will benefit from this change. Nice.
>
> It’s important to explain the background, what is being done, and why it
> is being done. And this commit message does all of that.
Hello Kristoffer,
Thank you very much for your response. |
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Sat, Oct 19, 2024 at 12:04 PM Kristoffer Haugsbakk
<[email protected]> wrote:
>
> Hi brian and Samuel
>
> On Sat, Oct 19, 2024, at 02:38, brian m. carlson wrote:
> > On 2024-10-19 at 00:14:13, Samuel Adekunle Abraham via GitGitGadget wrote:
> >> +test_expect_success '"git notes add" with -m/-F invokes the editor with -e' '
> >> + test_commit 19th &&
> >> + GIT_EDITOR="true" git notes add -m "note message" -e &&
> >> + git notes remove HEAD &&
> >> + echo "message from file" >file_1 &&
> >> + GIT_EDITOR="true" git notes add -F file_1 -e &&
> >> + git notes remove HEAD
> >> +'
> >
> > Maybe I don't understand what this is supposed to be testing (and if so,
> > please correct me), but how are we verifying that the editor is being
> > invoked? If we're invoking `true`, then that doesn't change the message
> > in any way, so if we suddenly stopped invoking the editor, I don't think
> > this would fail.
>
> I also didn’t understand these tests.
>
> There is this test in this file/test suite which tests the negative
> case:
>
> test_expect_success 'empty notes do not invoke the editor' '
> test_commit 18th &&
> GIT_EDITOR="false" git notes add -C "$empty_blob" --allow-empty &&
> git notes remove HEAD &&
> GIT_EDITOR="false" git notes add -m "" --allow-empty &&
> git notes remove HEAD &&
> GIT_EDITOR="false" git notes add -F /dev/null --allow-empty &&
> git notes remove HEAD
> '
>
Thank you Kristoffer,
Yes incorrectly used this as a reference and I have however look
deeper into the implementation
of the write_scripts function and the fake_editor file for better understanding.
> And this works because the commands would fail if the editor was invoked:
>
> error: there was a problem with the editor 'false'
>
> But this doesn’t work for `true`.
>
> > Maybe we could use something else as `GIT_EDITOR` instead. For example,
> > if we did `GIT_EDITOR="perl -pi -e s/file/editor/" git notes add -F file_1 -e`
> > (with an appropriate PERL prerequisite), then we could test that the
> > message after the fact was "message from editor", which would help us
> > verify that both the `-F` and `-e` options were being honoured.
> > (Similar things can be said about the tests you added below this as
> > well.)
>
> This file defines a `fake_editor`:[1]
>
> write_script fake_editor <<\EOF
> echo "$MSG" >"$1"
> echo "$MSG" >&2
> EOF
> GIT_EDITOR=./fake_editor
> export GIT_EDITOR
>
> And it looks like this is how it is used:
>
> test_expect_success 'create notes' '
> MSG=b4 git notes add &&
> test_path_is_missing .git/NOTES_EDITMSG &&
> git ls-tree -r refs/notes/commits >actual &&
> test_line_count = 1 actual &&
> echo b4 >expect &&
> git notes show >actual &&
> test_cmp expect actual &&
> git show HEAD^ &&
> test_must_fail git notes show HEAD^
> '
>
> So it seems that the new tests here should use the `test_cmp expect
> actual` style.
Thank you very much for the guide.
I will correct them and send a modified patch.
>
> † 1: The different test files use both `fake_editor`, `fake-editor`,
> and `fakeeditor`.
>
> > Do you think there are any cases where testing the `--no-edit`
> > functionality might be helpful? For example, is `git notes edit` ever
> > useful to invoke with such an option, like one might do with `git commit
> > amend`? (This isn't rhetorical, since the notes code is one of the areas
> > of Git I'm least familiar with, so I ask both because I'm curious and if
> > you think it's a useful thing to do, then tests might be a good idea.)
>
> Yes, that is useful (both as a use-case and as a regression test[2]).
> git-notes(1) is often used to programmatically add metadata:
>
> git show todo:post-applypatch | grep -C5 refs/notes/amlog
>
> (And this non-interactive example is not affected by this change since
> `-e` is required in order to invoke the editor)
>
> † 2: I seem to recall a regression in how git-notes(1) chose to invoke
> the editor or not |
|
On the Git mailing list, "Kristoffer Haugsbakk" wrote (reply to this): On Sat, Oct 19, 2024, at 02:14, Samuel Adekunle Abraham via GitGitGadget wrote:
> builtin/notes.c | 4 ++++
> t/t3301-notes.sh | 29 +++++++++++++++++++++++++++++
> 2 files changed, 33 insertions(+)
The documentation should be updated:
Documentation/git-notes.txt
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 8c26e455269..02cdfdf1c9d 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const
> char *prefix)
> OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> parse_reedit_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
The `add` subcommand does what I expect it to after some testing.
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv,
> const char *prefix)
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_BOOL(0, "allow-empty", &allow_empty,
> N_("allow storing empty note")),
> OPT_CALLBACK_F(0, "separator", &separator,
Likewise for the `append` subcommand.
--
Kristoffer Haugsbakk
|
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Sat, Oct 19, 2024 at 12:37 PM Kristoffer Haugsbakk
<[email protected]> wrote:
>
> On Sat, Oct 19, 2024, at 02:14, Samuel Adekunle Abraham via GitGitGadget wrote:
> > builtin/notes.c | 4 ++++
> > t/t3301-notes.sh | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 33 insertions(+)
>
> The documentation should be updated:
>
> Documentation/git-notes.txt
>
> > diff --git a/builtin/notes.c b/builtin/notes.c
> > index 8c26e455269..02cdfdf1c9d 100644
> > --- a/builtin/notes.c
> > +++ b/builtin/notes.c
> > @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const
> > char *prefix)
> > OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> > N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> > parse_reedit_arg),
> > + OPT_BOOL('e', "edit", &d.use_editor,
> > + N_("edit note message in editor")),
>
> The `add` subcommand does what I expect it to after some testing.
>
> > OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> > N_("reuse specified note object"), PARSE_OPT_NONEG,
> > parse_reuse_arg),
> > @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv,
> > const char *prefix)
> > OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> > N_("reuse specified note object"), PARSE_OPT_NONEG,
> > parse_reuse_arg),
> > + OPT_BOOL('e', "edit", &d.use_editor,
> > + N_("edit note message in editor")),
> > OPT_BOOL(0, "allow-empty", &allow_empty,
> > N_("allow storing empty note")),
> > OPT_CALLBACK_F(0, "separator", &separator,
>
> Likewise for the `append` subcommand.
>
> --
> Kristoffer Haugsbakk
>
Okay, I will do that. Thank you very much, Kristoffer. |
devdekunle
force-pushed
the
notes_add_e_option
branch
3 times, most recently
from
8082c0a
to
55804cd
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
@devdekunle please note that #1817 (comment), even if it was sent via email in a strict sense, still did not get sent to the Git mailing list... Could you please reply again, via email, to the actual email brian sent? |
Oh I see. Thanks I will reply again. |
|
On the Git mailing list, Patrick Steinhardt wrote (reply to this): On Sun, Oct 20, 2024 at 12:03:00AM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle <[email protected]>
>
> Notes can be added to a commit using the -m (message),
> -C (copy a note from a blob object) or
> -F (read the note from a file) options.
Nit: this would read a bit better if this was a bulleted list, I think.
E.g.:
Notes can be added to a commit using:
- "-m" to provide a message on the command line.
- -C to copy a note from a blob object.
- -F to read the note from a file.
When these options are used, ...
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
>
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and editted
s/editted/edited
> diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> index c9221a68cce..d5505a426aa 100644
> --- a/Documentation/git-notes.txt
> +++ b/Documentation/git-notes.txt
> @@ -9,9 +9,9 @@ SYNOPSIS
> --------
> [verse]
> 'git notes' [list [<object>]]
> -'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
> +'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>] [-e]
> 'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )
> -'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
> +'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>] [-e]
> 'git notes' edit [--allow-empty] [<object>] [--[no-]stripspace]
> 'git notes' show [<object>]
> 'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
Nit: I'd move the `[-e]` before [<object>] so that -F, -C and -m are all
close together.
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 8c26e455269..72c8a51cfac 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> parse_reedit_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> N_("reuse specified note object"), PARSE_OPT_NONEG,
> parse_reuse_arg),
> + OPT_BOOL('e', "edit", &d.use_editor,
> + N_("edit note message in editor")),
> OPT_BOOL(0, "allow-empty", &allow_empty,
> N_("allow storing empty note")),
> OPT_CALLBACK_F(0, "separator", &separator,
Nice.
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 99137fb2357..ffa1d21671d 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -1567,4 +1567,60 @@ test_expect_success 'empty notes do not invoke the editor' '
> git notes remove HEAD
> '
>
> +test_expect_success 'git notes add with -m/-F invokes editor with -e' '
> + test_commit 19th &&
> + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> + echo "Edited notes message" >expect &&
> + git notes show >actual &&
> + test_cmp expect actual &&
> + git notes remove HEAD &&
> +
> + # Add a note using -F and edit it
> + echo "Note from file" >note_file &&
> + MSG="Edited note from file" git notes add -F note_file -e &&
> + echo "Edited note from file" >expect &&
> + git notes show >actual &&
> + test_cmp expect actual
> +'
I was surprised at first why the MSG ended up in the commit message. But
the setup of t3301 writes a fake editor that listens to this environment
variable, so this looks good to me.
> +test_expect_success 'git notes append with -m/-F invokes the editor with -e' '
> + test_commit 20th &&
> + git notes add -m "Initial note message" &&
> + MSG="Appended edited note message" git notes append -m "New appended note" -e &&
> +
> + # Verify the note content was appended and edited
> + echo "Initial note message" >expect &&
> + echo "" >>expect &&
> + echo "Appended edited note message" >>expect &&
When you want to write multiple lines we typically use HERE docs. E.g.:
cat >expect <<-EOF &&
Initial note message
Appended edited note message
EOF
> + git notes show >actual &&
> + test_cmp expect actual &&
> + git notes remove HEAD &&
> +
> + #Append a note using -F and edit it
There should be a space after the "#" here.
> + echo "Note from file" >note_file &&
> + git notes add -m "Initial note message" &&
> + MSG="Appended edited note from file" git notes append -F note_file -e &&
> +
> + # Verify notes from file has been edited in editor and appended
> + echo "Initial note message" >expect &&
> + echo "" >>expect &&
> + echo "Appended edited note from file" >>expect &&
Same comment here.
> + git notes show >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'git notes with a combination of -m, -F and -e invokes editor' '
> + test_commit 21st &&
> + echo "foo-file-1" >note_1 &&
> + echo "foo-file-2" >note_2 &&
> +
> + MSG="Collapsed edited notes" git notes append -F note_1 -m "message-1" -F note_2 -e &&
> +
> + # Verify that combined messages from file and -m have been edited
> +
> + echo "Collapsed edited notes" >expect &&
> + git notes show >actual &&
> + test_cmp expect actual
> +'
> +
> test_done
It would be nice to have another test that uses EDITOR=false to
demonstrate that we abort when the editor returns an error.
Other than that this patch looks good to me, thanks a lot!
Patrick |
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Mon, Oct 21, 2024 at 12:58 PM Patrick Steinhardt <[email protected]> wrote:
>
> On Sun, Oct 20, 2024 at 12:03:00AM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > From: Abraham Samuel Adekunle <[email protected]>
> >
> > Notes can be added to a commit using the -m (message),
> > -C (copy a note from a blob object) or
> > -F (read the note from a file) options.
>
> Nit: this would read a bit better if this was a bulleted list, I think.
> E.g.:
>
> Notes can be added to a commit using:
>
> - "-m" to provide a message on the command line.
> - -C to copy a note from a blob object.
> - -F to read the note from a file.
>
> When these options are used, ...
Thank you Patrick. Noted.
>
> > When these options are used, Git does not open an editor,
> > it simply takes the content provided via these options and
> > attaches it to the commit as a note.
> >
> > Improve flexibility to fine-tune the note before finalizing it
> > by allowing the messages to be prefilled in the editor and editted
>
> s/editted/edited
Okay.
>
> > diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
> > index c9221a68cce..d5505a426aa 100644
> > --- a/Documentation/git-notes.txt
> > +++ b/Documentation/git-notes.txt
> > @@ -9,9 +9,9 @@ SYNOPSIS
> > --------
> > [verse]
> > 'git notes' [list [<object>]]
> > -'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
> > +'git notes' add [-f] [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>] [-e]
> > 'git notes' copy [-f] ( --stdin | <from-object> [<to-object>] )
> > -'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>]
> > +'git notes' append [--allow-empty] [--[no-]separator | --separator=<paragraph-break>] [--[no-]stripspace] [-F <file> | -m <msg> | (-c | -C) <object>] [<object>] [-e]
> > 'git notes' edit [--allow-empty] [<object>] [--[no-]stripspace]
> > 'git notes' show [<object>]
> > 'git notes' merge [-v | -q] [-s <strategy> ] <notes-ref>
>
> Nit: I'd move the `[-e]` before [<object>] so that -F, -C and -m are all
> close together.
>
> > diff --git a/builtin/notes.c b/builtin/notes.c
> > index 8c26e455269..72c8a51cfac 100644
> > --- a/builtin/notes.c
> > +++ b/builtin/notes.c
> > @@ -489,6 +489,8 @@ static int add(int argc, const char **argv, const char *prefix)
> > OPT_CALLBACK_F('c', "reedit-message", &d, N_("object"),
> > N_("reuse and edit specified note object"), PARSE_OPT_NONEG,
> > parse_reedit_arg),
> > + OPT_BOOL('e', "edit", &d.use_editor,
> > + N_("edit note message in editor")),
> > OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> > N_("reuse specified note object"), PARSE_OPT_NONEG,
> > parse_reuse_arg),
> > @@ -667,6 +669,8 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> > OPT_CALLBACK_F('C', "reuse-message", &d, N_("object"),
> > N_("reuse specified note object"), PARSE_OPT_NONEG,
> > parse_reuse_arg),
> > + OPT_BOOL('e', "edit", &d.use_editor,
> > + N_("edit note message in editor")),
> > OPT_BOOL(0, "allow-empty", &allow_empty,
> > N_("allow storing empty note")),
> > OPT_CALLBACK_F(0, "separator", &separator,
>
> Nice.
>
> > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> > index 99137fb2357..ffa1d21671d 100755
> > --- a/t/t3301-notes.sh
> > +++ b/t/t3301-notes.sh
> > @@ -1567,4 +1567,60 @@ test_expect_success 'empty notes do not invoke the editor' '
> > git notes remove HEAD
> > '
> >
> > +test_expect_success 'git notes add with -m/-F invokes editor with -e' '
> > + test_commit 19th &&
> > + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> > + echo "Edited notes message" >expect &&
> > + git notes show >actual &&
> > + test_cmp expect actual &&
> > + git notes remove HEAD &&
> > +
> > + # Add a note using -F and edit it
> > + echo "Note from file" >note_file &&
> > + MSG="Edited note from file" git notes add -F note_file -e &&
> > + echo "Edited note from file" >expect &&
> > + git notes show >actual &&
> > + test_cmp expect actual
> > +'
>
> I was surprised at first why the MSG ended up in the commit message. But
> the setup of t3301 writes a fake editor that listens to this environment
> variable, so this looks good to me.
>
> > +test_expect_success 'git notes append with -m/-F invokes the editor with -e' '
> > + test_commit 20th &&
> > + git notes add -m "Initial note message" &&
> > + MSG="Appended edited note message" git notes append -m "New appended note" -e &&
> > +
> > + # Verify the note content was appended and edited
> > + echo "Initial note message" >expect &&
> > + echo "" >>expect &&
> > + echo "Appended edited note message" >>expect &&
>
> When you want to write multiple lines we typically use HERE docs. E.g.:
>
> cat >expect <<-EOF &&
> Initial note message
>
> Appended edited note message
> EOF
>
> > + git notes show >actual &&
> > + test_cmp expect actual &&
> > + git notes remove HEAD &&
> > +
> > + #Append a note using -F and edit it
>
> There should be a space after the "#" here.
>
> > + echo "Note from file" >note_file &&
> > + git notes add -m "Initial note message" &&
> > + MSG="Appended edited note from file" git notes append -F note_file -e &&
> > +
> > + # Verify notes from file has been edited in editor and appended
> > + echo "Initial note message" >expect &&
> > + echo "" >>expect &&
> > + echo "Appended edited note from file" >>expect &&
>
> Same comment here.
>
> > + git notes show >actual &&
> > + test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git notes with a combination of -m, -F and -e invokes editor' '
> > + test_commit 21st &&
> > + echo "foo-file-1" >note_1 &&
> > + echo "foo-file-2" >note_2 &&
> > +
> > + MSG="Collapsed edited notes" git notes append -F note_1 -m "message-1" -F note_2 -e &&
> > +
> > + # Verify that combined messages from file and -m have been edited
> > +
> > + echo "Collapsed edited notes" >expect &&
> > + git notes show >actual &&
> > + test_cmp expect actual
> > +'
> > +
> > test_done
>
> It would be nice to have another test that uses EDITOR=false to
> demonstrate that we abort when the editor returns an error.
>
Thanks for the review, I will effect the changes.
> Other than that this patch looks good to me, thanks a lot!
You're welcome, Patrick.
>
> Patrick |
devdekunle
force-pushed
the
notes_add_e_option
branch
3 times, most recently
from
46e2e4d
to
8f80c61
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Taylor Blau wrote (reply to this): On Mon, Oct 21, 2024 at 02:38:15PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle <[email protected]>
>
> Notes can be added to a commit using:
> - "-m" to provide a message on the command line.
> - -C to copy a note from a blob object.
> - -F to read the note from a file.
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
>
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and edited
> after the messages have been provided through -[mF].
>
> Signed-off-by: Abraham Samuel Adekunle <[email protected]>
Nicely described, this commit message is looking good. Let's take a look
at the patch below...
> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> index 99137fb2357..2827f592c66 100755
> --- a/t/t3301-notes.sh
> +++ b/t/t3301-notes.sh
> @@ -1567,4 +1567,75 @@ test_expect_success 'empty notes do not invoke the editor' '
> git notes remove HEAD
> '
>
> +test_expect_success 'git notes add with -m/-F invokes editor with -e' '
> + test_commit 19th &&
> + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> + echo "Edited notes message" >expect &&
Very nice use of the fake_editor script here.
It is a little cumbersome to repeat the same message in MSG= and when
populating the 'expect' file. Perhaps instead this could be written as:
echo "edited notes message" >expect &&
MSG="$(cat expect)" git notes -add -m "initial" -e
> + git notes show >actual &&
> + test_cmp expect actual &&
> + git notes remove HEAD &&
> +
> + # Add a note using -F and edit it
> + echo "Note from file" >note_file &&
> + MSG="Edited note from file" git notes add -F note_file -e &&
> + echo "Edited note from file" >expect &&
Same "note" here. ;-).
> + git notes show >actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'git notes append with -m/-F invokes the editor with -e' '
> + test_commit 20th &&
> + git notes add -m "Initial note message" &&
> + MSG="Appended edited note message" git notes append -m "New appended note" -e &&
It's fine to use shorter values for -m and $MSG here. I think "appended"
and "edited" would be fine for each, respectively.
Besides applying those suggestions throughout the patch's new tests
(including the ones that I didn't explicitly comment on here), I think
that this should be looking good after another round. Thanks for working
on it.
Thanks,
Taylor |
|
User |
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Mon, Oct 21, 2024 at 5:53 PM Taylor Blau <[email protected]> wrote:
>
> On Mon, Oct 21, 2024 at 02:38:15PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > From: Abraham Samuel Adekunle <[email protected]>
> >
> > Notes can be added to a commit using:
> > - "-m" to provide a message on the command line.
> > - -C to copy a note from a blob object.
> > - -F to read the note from a file.
> > When these options are used, Git does not open an editor,
> > it simply takes the content provided via these options and
> > attaches it to the commit as a note.
> >
> > Improve flexibility to fine-tune the note before finalizing it
> > by allowing the messages to be prefilled in the editor and edited
> > after the messages have been provided through -[mF].
> >
> > Signed-off-by: Abraham Samuel Adekunle <[email protected]>
>
> Nicely described, this commit message is looking good. Let's take a look
> at the patch below...
>
> > diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
> > index 99137fb2357..2827f592c66 100755
> > --- a/t/t3301-notes.sh
> > +++ b/t/t3301-notes.sh
> > @@ -1567,4 +1567,75 @@ test_expect_success 'empty notes do not invoke the editor' '
> > git notes remove HEAD
> > '
> >
> > +test_expect_success 'git notes add with -m/-F invokes editor with -e' '
> > + test_commit 19th &&
> > + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> > + echo "Edited notes message" >expect &&
>
> Very nice use of the fake_editor script here.
>
> It is a little cumbersome to repeat the same message in MSG= and when
> populating the 'expect' file. Perhaps instead this could be written as:
>
> echo "edited notes message" >expect &&
> MSG="$(cat expect)" git notes -add -m "initial" -e
Concise! :-). Thank you very much Taylor.
>
> > + git notes show >actual &&
> > + test_cmp expect actual &&
> > + git notes remove HEAD &&
> > +
> > + # Add a note using -F and edit it
> > + echo "Note from file" >note_file &&
> > + MSG="Edited note from file" git notes add -F note_file -e &&
> > + echo "Edited note from file" >expect &&
>
> Same "note" here. ;-).
>
> > + git notes show >actual &&
> > + test_cmp expect actual
> > +'
> > +
> > +test_expect_success 'git notes append with -m/-F invokes the editor with -e' '
> > + test_commit 20th &&
> > + git notes add -m "Initial note message" &&
> > + MSG="Appended edited note message" git notes append -m "New appended note" -e &&
>
> It's fine to use shorter values for -m and $MSG here. I think "appended"
> and "edited" would be fine for each, respectively.
>
Okay. Noted
> Besides applying those suggestions throughout the patch's new tests
> (including the ones that I didn't explicitly comment on here), I think
> that this should be looking good after another round. Thanks for working
> on it.
Thank you very much,
Abraham Samuel
>
> Thanks,
> Taylor |
Notes can be added to a commit using: - "-m" to provide a message on the command line. - -C to copy a note from a blob object. - -F to read the note from a file. When these options are used, Git does not open an editor, it simply takes the content provided via these options and attaches it to the commit as a note. Improve flexibility to fine-tune the note before finalizing it by allowing the messages to be prefilled in the editor and edited after the messages have been provided through -[mF]. Signed-off-by: Abraham Samuel Adekunle <[email protected]>
devdekunle
force-pushed
the
notes_add_e_option
branch
from
8f80c61
to
4ecf79e
Compare
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Eric Sunshine wrote (reply to this): On Mon, Oct 21, 2024 at 8:00 AM Patrick Steinhardt <[email protected]> wrote:
> On Sun, Oct 20, 2024 at 12:03:00AM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > + echo "Initial note message" >expect &&
> > + echo "" >>expect &&
> > + echo "Appended edited note message" >>expect &&
>
> When you want to write multiple lines we typically use HERE docs. E.g.:
>
> cat >expect <<-EOF &&
> Initial note message
>
> Appended edited note message
> EOF
Nit: Since there are no variable interpolations inside the heredoc
body and we don't expect any, we'd normally indicate that by using
\EOF rather than EOF:
cat >expect <<-\EOF && |
|
User |
|
On the Git mailing list, Eric Sunshine wrote (reply to this): On Mon, Oct 21, 2024 at 12:53 PM Taylor Blau <[email protected]> wrote:
> On Mon, Oct 21, 2024 at 02:38:15PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> > + echo "Edited notes message" >expect &&
>
> Very nice use of the fake_editor script here.
>
> It is a little cumbersome to repeat the same message in MSG= and when
> populating the 'expect' file. Perhaps instead this could be written as:
>
> echo "edited notes message" >expect &&
> MSG="$(cat expect)" git notes -add -m "initial" -e
This suggested rewrite feels unusually roundabout and increases
cognitive load for readers who now have to trace the message flow from
script to file and back into script, and to consider how the loss of
trailing newline caused by use of $(...) impacts the behavior. It also
wastes a subprocess (which can be expensive on some platforms, such as
Windows). If we're really concerned about this minor duplication of
the message, we can instead do this:
msg="edited notes message" &&
echo "$msg" >expect &&
MSG="$msg" git notes -add -m "initial" -e |
|
On the Git mailing list, Taylor Blau wrote (reply to this): On Mon, Oct 21, 2024 at 02:28:05PM -0400, Eric Sunshine wrote:
> On Mon, Oct 21, 2024 at 12:53 PM Taylor Blau <[email protected]> wrote:
> > On Mon, Oct 21, 2024 at 02:38:15PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > > + MSG="Edited notes message" git notes add -m "Initial notes message" -e &&
> > > + echo "Edited notes message" >expect &&
> >
> > Very nice use of the fake_editor script here.
> >
> > It is a little cumbersome to repeat the same message in MSG= and when
> > populating the 'expect' file. Perhaps instead this could be written as:
> >
> > echo "edited notes message" >expect &&
> > MSG="$(cat expect)" git notes -add -m "initial" -e
>
> This suggested rewrite feels unusually roundabout and increases
> cognitive load for readers who now have to trace the message flow from
> script to file and back into script, and to consider how the loss of
> trailing newline caused by use of $(...) impacts the behavior. It also
> wastes a subprocess (which can be expensive on some platforms, such as
> Windows). If we're really concerned about this minor duplication of
> the message, we can instead do this:
>
> msg="edited notes message" &&
> echo "$msg" >expect &&
> MSG="$msg" git notes -add -m "initial" -e
I am not sure I agree that the suggested version is clearer. The way I
read mine is that we are writing what we expect to see in "expect", and
then setting up MSG to be the same thing.
I definitely do not feel strongly between the two and would rather avoid
nitpicking something as trivial as this when compared to the rest of the
patch, especially considering that I would be equally happy with your
version.
I think the whole thing is a little bit of a moot point, though, because
by far the thing that is least clear here is that setting MSG has the
side-effect of modifying the behavior of the fake-editor that we set up
earlier in the script. So I don't know that optimizing for clarity in
how we setup and write to the "expect" file is the right thing to spend
our time on.
Thanks,
Taylor |
|
On the Git mailing list, Taylor Blau wrote (reply to this): On Mon, Oct 21, 2024 at 06:12:20PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> From: Abraham Samuel Adekunle <[email protected]>
>
> Notes can be added to a commit using:
> - "-m" to provide a message on the command line.
> - -C to copy a note from a blob object.
> - -F to read the note from a file.
> When these options are used, Git does not open an editor,
> it simply takes the content provided via these options and
> attaches it to the commit as a note.
>
> Improve flexibility to fine-tune the note before finalizing it
> by allowing the messages to be prefilled in the editor and edited
> after the messages have been provided through -[mF].
>
> Signed-off-by: Abraham Samuel Adekunle <[email protected]>
Thanks, will queue.
Thanks,
Taylor |
|
On the Git mailing list, Samuel Abraham wrote (reply to this): On Mon, 21 Oct 2024, 20:53 Taylor Blau, <[email protected]> wrote:
>
> On Mon, Oct 21, 2024 at 06:12:20PM +0000, Samuel Adekunle Abraham via GitGitGadget wrote:
> > From: Abraham Samuel Adekunle <[email protected]>
> >
> > Notes can be added to a commit using:
> > - "-m" to provide a message on the command line.
> > - -C to copy a note from a blob object.
> > - -F to read the note from a file.
> > When these options are used, Git does not open an editor,
> > it simply takes the content provided via these options and
> > attaches it to the commit as a note.
> >
> > Improve flexibility to fine-tune the note before finalizing it
> > by allowing the messages to be prefilled in the editor and edited
> > after the messages have been provided through -[mF].
> >
> > Signed-off-by: Abraham Samuel Adekunle <[email protected]>
>
> Thanks, will queue.
>
Thank you, Taylor, my Outreachy mentors Patrick and Phillip, Junio,
and all the maintainers for your time and patience in reviewing
my patches. It has been a good learning period. Thanks once again.
Abraham Samuel
>
> Thanks,
> Taylor |
|
This patch series was integrated into seen via git@7173c1f. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Notes can be added to a commit using the -m (message), -C (copy a note from a blob object) or
-F (read the note from a file) options.
When these options are used, Git does not open an editor, it simply takes the content provided via these options and attaches it to the commit as a note.
Improve flexibility to fine-tune the note before finalizing it by allowing the messages to be prefilled in the editor and edited after they have been provided through -[mF].
cc: "brian m. carlson" [email protected]
cc: "Kristoffer Haugsbakk" [email protected]
cc: "Patrick Steinhardt" [email protected]
cc: "Phillip Wood" [email protected]
cc: "Junio C Hamano" [email protected]
cc: Taylor Blau [email protected]
cc: Eric Sunshine [email protected]