git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 00/11] several notes refs, post-rewrite, notes rewriting

From
Thomas Rast <trast@student.ethz.ch>
Date
Feb 23, 2010, 00:42 UTC
Message-ID
<cover.1266885599.git.trast@student.ethz.ch>
In-Reply-To
<cover.1266703765.git.trast@student.ethz.ch>
On Tuesday 23 February 2010 00:20:06 Junio C Hamano wrote:
> Thomas Rast <trast@student.ethz.ch> writes:
> > diff --git a/notes.c b/notes.c
[...]
> > +struct string_list display_notes_refs;
> > +struct notes_tree **display_notes_trees;
> 
> Do these need to be extern?

Don't think so? After all it's in the .c and just another static variable.

Show 6 quoted lines
> Nice use of strbuf_split().  I wonder if we should automatically add
> "refs/" and/or "refs/notes/" when the input is missing the prefix.  I
> don't have strong preference myself but the users might make noises.
> 
> Either way it needs to be documented in the final version before the
> series goes to 'master'.

I decided to do such dwimmery only on the command line, but not for the config. This is consistent with what's currently happening for core.notesRef. I also documented this.

Show 8 quoted lines
> > +static int notes_display_config(const char *k, const char *v, void *cb)
> > +{
> > +	/* Warning!  This is currently not executed if
> > +	 * GIT_NOTES_DISPLAY_REF is set.  Move the git_config() call
> > +	 * outside the test if you add more options. */
> 
> Yuck.  If you know what needs to be done, do that before other poeple add
> more options, please.
*shrug*

I was trying to be smart and save a call to git_config() when we know we don't care about the config anyway. After all it does read a bunch of files.

> >> > +	if (flags & NOTES_SHOW_HEADER_WITH_REF && t->ref) {
[...]
Show 8 quoted lines
> >> > +	} else if (flags & (NOTES_SHOW_HEADER|NOTES_SHOW_HEADER_WITH_REF))
> >> >  		strbuf_addstr(sb, "\nNotes:\n");
> 
> Meaning nobody will go through the latter "Notes:\n" codepath?  Then what
> is that else clause for?
> 
> Perhaps I am not reading your code right in which case this part needs a
> bit more commenting?

The catch is in the '&& t->ref'. Again I'm just trying to be a bit defensive. Anyway, I ripped out the old formatting and put the NULL test first this time.

Show 10 quoted lines
> >> I expected to see "Notes:\n" regardless of the mode if the notes is coming
> >> from the default refs/notes/commits tree, but it probably is better to say
> >> "Notes (commits):\n" like your patch does.
> >
> > I special-cased GIT_NOTES_DEFAULT_REF (which is "refs/notes/commits")
> > above *at your request* to not change the output in the default case.
> 
> > So which way do you want it?
> 
> I don't have strong preference anymore with the above code.
[...]
> But if everybody calls with HEADER_WITH_REF, no matter what the end user
> preference is, then it becomes unclear what the right answer would be.

Well, I think we're into bikeshedding territory now, so I just left it as it was.

The rest of the changes are listed in each email.
Thomas Rast (11):
  test-lib: unset GIT_NOTES_REF to stop it from influencing tests
  Support showing notes from more than one notes tree
  Documentation: document post-rewrite hook
  commit --amend: invoke post-rewrite hook
  rebase: invoke post-rewrite hook
  rebase -i: invoke post-rewrite hook
  notes: implement 'git notes copy --stdin'
  notes: implement helpers needed for note copying during rewrite
  rebase: support automatic notes copying
  commit --amend: copy notes to the new commit
  notes: add shorthand --ref to override GIT_NOTES_REF
 Documentation/config.txt         |   53 +++++-
 Documentation/git-notes.txt      |   21 ++-
 Documentation/githooks.txt       |   38 ++++
 Documentation/pretty-options.txt |   11 +-
 builtin-commit.c                 |   45 +++++
 builtin-log.c                    |    2 +
 builtin-notes.c                  |  195 +++++++++++++++++++-
 builtin.h                        |   18 ++
 cache.h                          |    3 +
 git-am.sh                        |   13 ++
 git-rebase--interactive.sh       |   52 +++++-
 git-rebase.sh                    |    6 +
 notes.c                          |  191 +++++++++++++++++++-
 notes.h                          |   27 +++
 pretty.c                         |    6 +-
 refs.c                           |    4 +-
 refs.h                           |    5 +
 revision.c                       |   21 ++
 revision.h                       |    5 +
 t/t3301-notes.sh                 |  377 +++++++++++++++++++++++++++++++++++++-
 t/t3400-rebase.sh                |   17 ++
 t/t3404-rebase-interactive.sh    |   24 +++
 t/t5407-post-rewrite-hook.sh     |  172 +++++++++++++++++
 t/t7501-commit.sh                |   12 ++
 t/test-lib.sh                    |    4 +
 25 files changed, 1294 insertions(+), 28 deletions(-)
 create mode 100755 t/t5407-post-rewrite-hook.sh
Previous: Junio C HamanoNext: Thomas Rast
Message 21 of 66 in “several notes refs, post-rewrite, notes rewriting”
  1. 00/12 several notes refs, post-rewrite, notes rewritingThomas Rast, Feb 20, 2010
  2. 01/12 Support showing notes from more than one notes treeThomas Rast, Feb 20, 2010
  3. Junio C HamanoFeb 21, 2010
  4. 02/12 Documentation: document post-rewrite hookThomas Rast, Feb 20, 2010
  5. 03/12 commit --amend: invoke post-rewrite hookThomas Rast, Feb 20, 2010
  6. Junio C HamanoFeb 21, 2010
  7. 04/12 rebase: invoke post-rewrite hookThomas Rast, Feb 20, 2010
  8. 05/12 rebase -i: invoke post-rewrite hookThomas Rast, Feb 20, 2010
  9. 06/12 notes: implement 'git notes copy --stdin'Thomas Rast, Feb 20, 2010
  10. Junio C HamanoFeb 21, 2010
  11. 07/12 notes: implement helpers needed for note copying during rewriteThomas Rast, Feb 20, 2010
  12. Junio C HamanoFeb 21, 2010
  13. 08/12 rebase: support automatic notes copyingThomas Rast, Feb 20, 2010
  14. 09/12 commit --amend: copy notes to the new commitThomas Rast, Feb 20, 2010
  15. 10/12 filter-branch: invoke post-rewrite hookThomas Rast, Feb 20, 2010
  16. 11/12 filter-branch: learn how to filter notesThomas Rast, Feb 20, 2010
  17. 12/12 notes: add shorthand --ref to override GIT_NOTES_REFThomas Rast, Feb 20, 2010
  18. Junio C HamanoFeb 21, 2010
  19. Thomas RastFeb 21, 2010
  20. Junio C HamanoFeb 22, 2010
  21. 00/11 several notes refs, post-rewrite, notes rewritingThomas Rast, Feb 23, 2010
  22. 01/11 test-lib: unset GIT_NOTES_REF to stop it from influencing testsThomas Rast, Feb 23, 2010
  23. 02/11 Support showing notes from more than one notes treeThomas Rast, Feb 23, 2010
  24. Junio C HamanoFeb 23, 2010
  25. Thomas RastFeb 23, 2010
  26. format-patch: learn to fill comment section of email from notesThomas Rast, Feb 23, 2010
  27. BROKEN -- format-patch: learn to fill comment section of email from notesThomas Rast, Feb 23, 2010
  28. Thomas RastFeb 23, 2010
  29. Stephen BoydFeb 24, 2010
  30. Junio C HamanoFeb 23, 2010
  31. Thomas RastMar 10, 2010
  32. 03/11 Documentation: document post-rewrite hookThomas Rast, Feb 23, 2010
  33. 04/11 commit --amend: invoke post-rewrite hookThomas Rast, Feb 23, 2010
  34. 05/11 rebase: invoke post-rewrite hookThomas Rast, Feb 23, 2010
  35. 06/11 rebase -i: invoke post-rewrite hookThomas Rast, Feb 23, 2010
  36. Junio C HamanoFeb 24, 2010
  37. 07/11 notes: implement 'git notes copy --stdin'Thomas Rast, Feb 23, 2010
  38. 08/11 notes: implement helpers needed for note copying during rewriteThomas Rast, Feb 23, 2010
  39. 09/11 rebase: support automatic notes copyingThomas Rast, Feb 23, 2010
  40. Junio C HamanoFeb 25, 2010
  41. 00/13 several notes refs, post-rewrite, notes rewritingThomas Rast, Mar 10, 2010
  42. 01/13 test-lib: unset GIT_NOTES_REF to stop it from influencing testsThomas Rast, Mar 10, 2010
  43. Johan HerlandMar 11, 2010
  44. 02/13 Support showing notes from more than one notes treeThomas Rast, Mar 10, 2010
  45. Johan HerlandMar 11, 2010
  46. 03/13 Documentation: document post-rewrite hookThomas Rast, Mar 10, 2010
  47. 04/13 commit --amend: invoke post-rewrite hookThomas Rast, Mar 10, 2010
  48. 05/13 rebase: invoke post-rewrite hookThomas Rast, Mar 10, 2010
  49. 06/13 rebase -i: invoke post-rewrite hookThomas Rast, Mar 10, 2010
  50. 07/13 notes: implement 'git notes copy --stdin'Thomas Rast, Mar 10, 2010
  51. Johan HerlandMar 11, 2010
  52. 08/13 notes: implement helpers needed for note copying during rewriteThomas Rast, Mar 10, 2010
  53. Johan HerlandMar 11, 2010
  54. 09/13 rebase: support automatic notes copyingThomas Rast, Mar 10, 2010
  55. 10/13 commit --amend: copy notes to the new commitThomas Rast, Mar 10, 2010
  56. 11/13 notes: add shorthand --ref to override GIT_NOTES_REFThomas Rast, Mar 10, 2010
  57. Johan HerlandMar 11, 2010
  58. 12/13 notes: track whether notes_trees were changed at allThomas Rast, Mar 10, 2010
  59. Johan HerlandMar 11, 2010
  60. 13/13 git-notes(1): add a section about the meaning of historyThomas Rast, Mar 10, 2010
  61. Johan HerlandMar 11, 2010
  62. Junio C HamanoMar 10, 2010
  63. 10/11 commit --amend: copy notes to the new commitThomas Rast, Feb 23, 2010
  64. 11/11 notes: add shorthand --ref to override GIT_NOTES_REFThomas Rast, Feb 23, 2010
  65. Junio C HamanoFeb 23, 2010
  66. Thomas RastFeb 23, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.