git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Martin Ågren" <martin.agren@gmail.com>
Subject: [PATCH v2 0/8] parsing trailers with shortlog
Date: Sun, 27 Sep 2020 04:39:33 -0400	[thread overview]
Message-ID: <20200927083933.GA2222823@coredump.intra.peff.net> (raw)
In-Reply-To: <20200925070120.GA3669667@coredump.intra.peff.net>

On Fri, Sep 25, 2020 at 03:01:20AM -0400, Jeff King wrote:

> Somebody mentioned at the inclusion summit that it would be nice for
> mentoring/pairing relationships if we could more easily give credit to
> multiple authors of a commit. When people ask about adding multiple
> "author" headers, we usually recommend that they use a "co-authored-by"
> trailer. But you can't convince shortlog to count it for anything. :)
> 
> So this series adds support for counting trailers to shortlog. You can
> do a number of fun things with it, like:

Here's a re-roll with some cosmetic fixups. All except the first address
the points raised by Eric and Martin (thanks, both):

  - put the --group=<type> unordered list into a delimited block, so
    that the follow-on paragraph about multiple --group options isn't
    indented with "trailer" item

  - reword --group docs for clarity

  - use existing HAS_MULTI_BITS

  - add trailing commas in enum

  - complete truncated trailer_iterator comment

  - fix misleading "refactor" commit message in patch 2

Range diff is below.

  [1/8]: shortlog: change "author" variables to "ident"
  [2/8]: shortlog: add grouping option
  [3/8]: trailer: add interface for iterating over commit trailers
  [4/8]: shortlog: match commit trailers with --group
  [5/8]: shortlog: de-duplicate trailer values
  [6/8]: shortlog: rename parse_stdin_ident()
  [7/8]: shortlog: parse trailer idents
  [8/8]: shortlog: allow multiple groups to be specified

 Documentation/git-shortlog.txt |  31 ++++-
 builtin/log.c                  |   1 +
 builtin/shortlog.c             | 213 ++++++++++++++++++++++++++++-----
 shortlog.h                     |   8 +-
 t/t4201-shortlog.sh            | 141 ++++++++++++++++++++++
 trailer.c                      |  36 ++++++
 trailer.h                      |  45 ++++++-
 7 files changed, 444 insertions(+), 31 deletions(-)

1:  675e8cea10 = 1:  3b658cba7c shortlog: change "author" variables to "ident"
2:  72d94a2601 ! 2:  3ed8a55597 shortlog: refactor committer/author grouping
    @@ Metadata
     Author: Jeff King <peff@peff.net>
     
      ## Commit message ##
    -    shortlog: refactor committer/author grouping
    +    shortlog: add grouping option
     
    -    In preparation for adding more grouping types, let's
    -    refactor the committer/author grouping code. In particular:
    +    In preparation for adding more grouping types, let's refactor the
    +    committer/author grouping code and add a user-facing option that binds
    +    them together. In particular:
     
    -      - the master option is now "--group", to make it clear
    +      - the main option is now "--group", to make it clear
             that the various group types are mutually exclusive. The
             "--committer" option is an alias for "--group=committer".
     
    @@ Documentation/git-shortlog.txt: OPTIONS
      	Each pretty-printed commit will be rewrapped before it is shown.
      
     +--group=<type>::
    -+	By default, `shortlog` collects and collates author identities;
    -+	using `--group` will collect other fields.
    -+	`<type>` is one of:
    ++	Group commits based on `<type>`. If no `--group` option is
    ++	specified, the default is `author`. `<type>` is one of:
     ++
    -+ - `author`, commits are grouped by author (this is the default)
    ++ - `author`, commits are grouped by author
     + - `committer`, commits are grouped by committer (the same as `-c`)
     +
      -c::
      --committer::
    - 	Collect and show committer identities instead of authors.
    +-	Collect and show committer identities instead of authors.
     +	This is an alias for `--group=committer`.
      
      -w[<width>[,<indent1>[,<indent2>]]]::
    @@ shortlog.h: struct shortlog {
     +
     +	enum {
     +		SHORTLOG_GROUP_AUTHOR = 0,
    -+		SHORTLOG_GROUP_COMMITTER
    ++		SHORTLOG_GROUP_COMMITTER,
     +	} group;
      
      	char *common_repo_prefix;
3:  bec4ec1433 ! 3:  d314578bde trailer: add interface for iterating over commit trailers
    @@ trailer.h: void trailer_info_release(struct trailer_info *info);
     + * message "msg". The "msg" pointer must remain valid until the iterator is
     + * released.
     + *
    -+ * After initializing, we are not yet pointing
    ++ * After initializing, note that key/val will not yet point to any trailer.
    ++ * Call advance() to parse the first one (if any).
     + */
     +void trailer_iterator_init(struct trailer_iterator *iter, const char *msg);
     +
4:  7e6ee8317f ! 4:  fdb1dbd164 shortlog: match commit trailers with --group
    @@ Commit message
      ## Documentation/git-shortlog.txt ##
     @@ Documentation/git-shortlog.txt: OPTIONS
      +
    -  - `author`, commits are grouped by author (this is the default)
    +  - `author`, commits are grouped by author
       - `committer`, commits are grouped by committer (the same as `-c`)
     + - `trailer:<field>`, the `<field>` is interpreted as a case-insensitive
     +   commit message trailer (see linkgit:git-interpret-trailers[1]). For
    @@ builtin/shortlog.c: static int parse_wrap_args(const struct option *opt, const c
     
      ## shortlog.h ##
     @@ shortlog.h: struct shortlog {
    - 
      	enum {
      		SHORTLOG_GROUP_AUTHOR = 0,
    --		SHORTLOG_GROUP_COMMITTER
    -+		SHORTLOG_GROUP_COMMITTER,
    -+		SHORTLOG_GROUP_TRAILER
    + 		SHORTLOG_GROUP_COMMITTER,
    ++		SHORTLOG_GROUP_TRAILER,
      	} group;
     +	char *trailer;
      
5:  8bc6a94239 = 5:  6374604756 shortlog: de-duplicate trailer values
6:  16294b5d16 = 6:  dfabab9957 shortlog: rename parse_stdin_ident()
7:  82c637c0a2 = 7:  bbe87d0e2a shortlog: parse trailer idents
8:  4cacdecaa0 ! 8:  ca782fa92e shortlog: allow multiple groups to be specified
    @@ Commit message
         Signed-off-by: Jeff King <peff@peff.net>
     
      ## Documentation/git-shortlog.txt ##
    +@@ Documentation/git-shortlog.txt: OPTIONS
    + 	Group commits based on `<type>`. If no `--group` option is
    + 	specified, the default is `author`. `<type>` is one of:
    + +
    ++--
    +  - `author`, commits are grouped by author
    +  - `committer`, commits are grouped by committer (the same as `-c`)
    +  - `trailer:<field>`, the `<field>` is interpreted as a case-insensitive
     @@ Documentation/git-shortlog.txt: Shortlog will attempt to parse each trailer value as a `name <email>`
      identity. If successful, the mailmap is applied and the email is omitted
      unless the `--email` option is specified. If the value cannot be parsed
      as an identity, it will be taken literally and completely.
    ++--
     ++
     +If `--group` is specified multiple times, commits are counted under each
     +value (but again, only once per unique value in that commit). For
    @@ builtin/log.c: static void make_cover_letter(struct rev_info *rev, int use_stdou
      
     
      ## builtin/shortlog.c ##
    -@@ builtin/shortlog.c: static int parse_ident(struct shortlog *log,
    - 	return 0;
    - }
    - 
    -+static inline int has_multi_bits(unsigned n)
    -+{
    -+	return (n & (n - 1)) != 0;
    -+}
    -+
    - static void read_from_stdin(struct shortlog *log)
    - {
    - 	struct strbuf ident = STRBUF_INIT;
     @@ builtin/shortlog.c: static void read_from_stdin(struct shortlog *log)
      	static const char *committer_match[2] = { "Commit: ", "committer " };
      	const char **match;
      
     -	switch (log->group) {
    -+	if (has_multi_bits(log->groups))
    ++	if (HAS_MULTI_BITS(log->groups))
     +		die(_("using multiple --group options with stdin is not supported"));
     +
     +	switch (log->groups) {
    @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit
     -		insert_one_record(log, ident.buf, oneline_str);
     -		break;
     -	case SHORTLOG_GROUP_COMMITTER:
    -+		if (!has_multi_bits(log->groups) ||
    ++		if (!HAS_MULTI_BITS(log->groups) ||
     +		    !strset_check_and_add(&dups, ident.buf))
     +			insert_one_record(log, ident.buf, oneline_str);
     +	}
    @@ builtin/shortlog.c: void shortlog_add_commit(struct shortlog *log, struct commit
     -	case SHORTLOG_GROUP_TRAILER:
     -		insert_records_from_trailers(log, commit, &ctx, oneline_str);
     -		break;
    -+		if (!has_multi_bits(log->groups) ||
    ++		if (!HAS_MULTI_BITS(log->groups) ||
     +		    !strset_check_and_add(&dups, ident.buf))
     +			insert_one_record(log, ident.buf, oneline_str);
     +	}
    @@ shortlog.h: struct shortlog {
      	enum {
     -		SHORTLOG_GROUP_AUTHOR = 0,
     -		SHORTLOG_GROUP_COMMITTER,
    --		SHORTLOG_GROUP_TRAILER
    +-		SHORTLOG_GROUP_TRAILER,
     -	} group;
     -	char *trailer;
     +		SHORTLOG_GROUP_AUTHOR = (1 << 0),
     +		SHORTLOG_GROUP_COMMITTER = (1 << 1),
    -+		SHORTLOG_GROUP_TRAILER = (1 << 2)
    ++		SHORTLOG_GROUP_TRAILER = (1 << 2),
     +	} groups;
     +	struct string_list trailers;
      

  parent reply	other threads:[~2020-09-27  8:39 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-25  7:01 [PATCH " Jeff King
2020-09-25  7:01 ` [PATCH 1/8] shortlog: change "author" variables to "ident" Jeff King
2020-09-25  7:02 ` [PATCH 2/8] shortlog: refactor committer/author grouping Jeff King
2020-09-25 20:05   ` Eric Sunshine
2020-09-27  8:03     ` Jeff King
2020-09-27  8:08       ` Jeff King
2020-09-27  8:23         ` Eric Sunshine
2020-09-26 12:31   ` Martin Ågren
2020-09-27  7:59     ` Jeff King
2020-09-25  7:02 ` [PATCH 3/8] trailer: add interface for iterating over commit trailers Jeff King
2020-09-26 12:39   ` Martin Ågren
2020-09-27  8:20     ` Jeff King
2020-09-25  7:03 ` [PATCH 4/8] shortlog: match commit trailers with --group Jeff King
2020-09-25  7:05 ` [PATCH 5/8] shortlog: de-duplicate trailer values Jeff King
2020-09-25  7:05 ` [PATCH 6/8] shortlog: rename parse_stdin_ident() Jeff King
2020-09-25  7:05 ` [PATCH 7/8] shortlog: parse trailer idents Jeff King
2020-09-25  7:05 ` [PATCH 8/8] shortlog: allow multiple groups to be specified Jeff King
2020-09-25 20:23   ` Eric Sunshine
2020-09-27  8:06     ` Jeff King
2020-09-26 12:48   ` Martin Ågren
2020-09-27  8:25     ` Jeff King
2020-09-25 14:27 ` [PATCH 0/8] parsing trailers with shortlog Derrick Stolee
2020-09-25 16:57 ` Junio C Hamano
2020-09-27  8:39 ` Jeff King [this message]
2020-09-27  8:39   ` [PATCH v2 1/8] shortlog: change "author" variables to "ident" Jeff King
2020-09-27 19:18     ` Junio C Hamano
2020-09-27  8:39   ` [PATCH v2 2/8] shortlog: add grouping option Jeff King
2020-09-27  8:40   ` [PATCH v2 3/8] trailer: add interface for iterating over commit trailers Jeff King
2020-09-27  8:40   ` [PATCH v2 4/8] shortlog: match commit trailers with --group Jeff King
2020-09-27 19:51     ` Junio C Hamano
2020-09-28  3:17       ` Jeff King
2020-09-28 17:01         ` Junio C Hamano
2020-09-27  8:40   ` [PATCH v2 5/8] shortlog: de-duplicate trailer values Jeff King
2020-09-27 20:23     ` Junio C Hamano
2020-09-28  3:19       ` Jeff King
2020-09-27  8:40   ` [PATCH v2 6/8] shortlog: rename parse_stdin_ident() Jeff King
2020-09-27  8:40   ` [PATCH v2 7/8] shortlog: parse trailer idents Jeff King
2020-09-27 20:49     ` Junio C Hamano
2020-09-27  8:40   ` [PATCH v2 8/8] shortlog: allow multiple groups to be specified Jeff King
2020-09-27 21:18     ` Junio C Hamano
2020-09-28  3:25       ` Jeff King
2020-12-28 11:29     ` Junio C Hamano
2021-02-04  6:44       ` Junio C Hamano
2020-09-27 14:38   ` [PATCH v2 0/8] parsing trailers with shortlog Martin Ågren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200927083933.GA2222823@coredump.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=martin.agren@gmail.com \
    --cc=sunshine@sunshineco.com \
    --subject='Re: [PATCH v2 0/8] parsing trailers with shortlog' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).