| Message ID | 20241023201430.986389-1-bence@ferdinandy.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [RFC] notes: add prepend command | expand |
On Wed, Oct 23, 2024 at 10:14:24PM +0200, Bence Ferdinandy wrote: > When a note is detailing commit history, it makes sense to keep the > latest change on top, but unlike adding things at the bottom with > "git notes append" this can only be done manually. Add a > > git notes prepend > > command, which works exactly like the append command, except that it > inserts the text before the current contents of the note instead of > after. Hmmm. I am not sure that I see the widespread need for such a tool. If this is specific to your use-case, I think a custom script and `$GIT_EDITOR` would do the trick. Thanks, Taylor
On Wed Oct 23, 2024 at 22:20, Taylor Blau <me@ttaylorr.com> wrote: > On Wed, Oct 23, 2024 at 10:14:24PM +0200, Bence Ferdinandy wrote: >> When a note is detailing commit history, it makes sense to keep the >> latest change on top, but unlike adding things at the bottom with >> "git notes append" this can only be done manually. Add a >> >> git notes prepend >> >> command, which works exactly like the append command, except that it >> inserts the text before the current contents of the note instead of >> after. > > Hmmm. I am not sure that I see the widespread need for such a tool. If > this is specific to your use-case, I think a custom script and > `$GIT_EDITOR` would do the trick. Couldn't the same argument be made for append? Imho, it's a missing symmetry. Ofc it's probably not quite hard to script around this.
On 2024-10-23 22:14:24+0200, Bence Ferdinandy <bence@ferdinandy.com> wrote: > -static int append_edit(int argc, const char **argv, const char *prefix) > + > +static int append_prepend_edit(int argc, const char **argv, const char *prefix, int prepend) > { > int allow_empty = 0; > const char *object_ref; > @@ -716,11 +718,18 @@ static int append_edit(int argc, const char **argv, const char *prefix) > > if (!prev_buf) > die(_("unable to read %s"), oid_to_hex(note)); > - if (size) > - strbuf_add(&buf, prev_buf, size); > - if (d.buf.len && size) > - append_separator(&buf); > - strbuf_insert(&d.buf, 0, buf.buf, buf.len); > + if (prepend) { > + if (d.buf.len && size) > + append_separator(&buf); > + if (size) > + strbuf_add(&buf, prev_buf, size); > + } else { > + if (size) > + strbuf_add(&buf, prev_buf, size); > + if (d.buf.len && size) > + append_separator(&buf); > + } > + strbuf_insert(&d.buf, prepend ? d.buf.len : 0, buf.buf, buf.len); > > free(prev_buf); > strbuf_release(&buf); Without prejudice about whether we should take this command. (I think we shouldn't, just like we shouldn't top-posting). I think this diff should be written like this for easier reasoning: ----- 8< ----------------- @@ -711,19 +713,27 @@ static int append_edit(int argc, const char **argv, const char *prefix) /* Append buf to previous note contents */ unsigned long size; enum object_type type; - struct strbuf buf = STRBUF_INIT; char *prev_buf = repo_read_object_file(the_repository, note, &type, &size); if (!prev_buf) die(_("unable to read %s"), oid_to_hex(note)); - if (size) + if (!size) { + // no existing notes, use whatever we have here + } else if (prepend) { + if (d.buf.len) + append_separator(&d.buf); + strbuf_add(&d.buf, prev_buf, size); + } else { + struct strbuf buf = STRBUF_INIT; strbuf_add(&buf, prev_buf, size); - if (d.buf.len && size) - append_separator(&buf); - strbuf_insert(&d.buf, 0, buf.buf, buf.len); + if (d.buf.len) + append_separator(&buf); + strbuf_addbuf(&buf, &d.buf); + strbuf_swap(&buf, &d.buf); + strbuf_release(&buf); + } free(prev_buf); - strbuf_release(&buf); } if (d.buf.len || allow_empty) { -------------- 8< -------------------- Even if we don't take this subcommand, I think we should re-write the append part, so: - we can see the append logic better, - we can avoid the `strbuf_insert` which will require memmove/memcpy. Well, the second point is micro-optimisation, so take it with a grain of salt. Also tests. -------------- 8< ----------------------- diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh index 99137fb235731..5a7ad40fde6a8 100755 --- a/t/t3301-notes.sh +++ b/t/t3301-notes.sh @@ -558,6 +558,20 @@ test_expect_success 'listing non-existing notes fails' ' test_must_be_empty actual ' +test_expect_success 'append: specify a separator with an empty arg' ' + test_when_finished git notes remove HEAD && + cat >expect <<-\EOF && + notes-2 + + notes-1 + EOF + + git notes add -m "notes-1" && + git notes prepend --separator="" -m "notes-2" && + git notes show >actual && + test_cmp expect actual +' + test_expect_success 'append: specify a separator with an empty arg' ' test_when_finished git notes remove HEAD && cat >expect <<-\EOF && ----------- >8 --------------
diff --git a/builtin/notes.c b/builtin/notes.c index 8c26e45526..cf158cab1c 100644 --- a/builtin/notes.c +++ b/builtin/notes.c @@ -35,6 +35,7 @@ static const char * const git_notes_usage[] = { N_("git notes [--ref <notes-ref>] add [-f] [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [--[no-]stripspace] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"), N_("git notes [--ref <notes-ref>] copy [-f] <from-object> <to-object>"), N_("git notes [--ref <notes-ref>] append [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [--[no-]stripspace] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"), + N_("git notes [--ref <notes-ref>] prepend [--allow-empty] [--[no-]separator|--separator=<paragraph-break>] [--[no-]stripspace] [-m <msg> | -F <file> | (-c | -C) <object>] [<object>]"), N_("git notes [--ref <notes-ref>] edit [--allow-empty] [<object>]"), N_("git notes [--ref <notes-ref>] show [<object>]"), N_("git notes [--ref <notes-ref>] merge [-v | -q] [-s <strategy>] <notes-ref>"), @@ -644,7 +645,8 @@ static int copy(int argc, const char **argv, const char *prefix) return retval; } -static int append_edit(int argc, const char **argv, const char *prefix) + +static int append_prepend_edit(int argc, const char **argv, const char *prefix, int prepend) { int allow_empty = 0; const char *object_ref; @@ -716,11 +718,18 @@ static int append_edit(int argc, const char **argv, const char *prefix) if (!prev_buf) die(_("unable to read %s"), oid_to_hex(note)); - if (size) - strbuf_add(&buf, prev_buf, size); - if (d.buf.len && size) - append_separator(&buf); - strbuf_insert(&d.buf, 0, buf.buf, buf.len); + if (prepend) { + if (d.buf.len && size) + append_separator(&buf); + if (size) + strbuf_add(&buf, prev_buf, size); + } else { + if (size) + strbuf_add(&buf, prev_buf, size); + if (d.buf.len && size) + append_separator(&buf); + } + strbuf_insert(&d.buf, prepend ? d.buf.len : 0, buf.buf, buf.len); free(prev_buf); strbuf_release(&buf); @@ -745,6 +754,16 @@ static int append_edit(int argc, const char **argv, const char *prefix) return 0; } +static int prepend_edit(int argc, const char **argv, const char *prefix) +{ + return append_prepend_edit(argc, argv, prefix, 1); +} + +static int append_edit(int argc, const char **argv, const char *prefix) +{ + return append_prepend_edit(argc, argv, prefix, 0); +} + static int show(int argc, const char **argv, const char *prefix) { const char *object_ref; @@ -1116,6 +1135,7 @@ int cmd_notes(int argc, OPT_SUBCOMMAND("add", &fn, add), OPT_SUBCOMMAND("copy", &fn, copy), OPT_SUBCOMMAND("append", &fn, append_edit), + OPT_SUBCOMMAND("prepend", &fn, prepend_edit), OPT_SUBCOMMAND("edit", &fn, append_edit), OPT_SUBCOMMAND("show", &fn, show), OPT_SUBCOMMAND("merge", &fn, merge),
When a note is detailing commit history, it makes sense to keep the latest change on top, but unlike adding things at the bottom with "git notes append" this can only be done manually. Add a git notes prepend command, which works exactly like the append command, except that it inserts the text before the current contents of the note instead of after. Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com> --- Notes: RFC v1: Cf. https://lore.kernel.org/git/20241023153736.257733-1-bence@ferdinandy.com/T/#m5b6644827590c2518089ab84f936a970c4e9be0f For that particular series I've used git rev-list HEAD~8..HEAD | xargs -i git notes append {} -m "v12: no change" for a quick-start on updating notes, when only 1 note needed to be really edited with meaningful content, and for some of the patches you now need to scroll a bit to actually find that "no change" text, instead of seeing it right at the top. builtin/notes.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)