public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Junio C Hamano <gitster@pobox•com>
To: Koosha Khajehmoogahi <koosha@posteo•de>
Cc: git@vger•kernel.org, sunshine@sunshineco•com
Subject: Re: [PATCH v2 2/5] log: honor log.merges= option
Date: Sat, 04 Apr 2015 13:00:33 -0700	[thread overview]
Message-ID: <xmqqy4m7ek9q.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1428110521-31028-2-git-send-email-koosha@posteo.de> (Koosha Khajehmoogahi's message of "Sat, 4 Apr 2015 03:21:58 +0200")

Koosha Khajehmoogahi <koosha@posteo•de> writes:

> From: Junio C Hamano <gitster@pobox•com>
>
> [kk: wrote commit message]

Ehh, what exactly did you write ;-)?

I think the most important thing that needs to be explained by the
log message for this change is that the variable is honored only by
log and it needs to explain why other Porcelain commands in the same
"log" family, like "whatchanged", should ignore the variable.

I think that we must not to allow format-patch and show to be
affected by this variable, because it is silly if log.merges=only
broke format-patch output or made "git show" silent.  But I didn't
think about others.  Whoever is doing this change needs to explain
in the log message the reason why it was decided that only "git log"
should pay attention to it.

> Helped-by: Eris Sunshine <sunshine@sunshineco•com>
> Signed-off-by: Koosha Khajehmoogahi <koosha@posteo•de>
> ---


>  builtin/log.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index dd8f3fc..c7a7aad 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -36,6 +36,7 @@ static int decoration_given;
>  static int use_mailmap_config;
>  static const char *fmt_patch_subject_prefix = "PATCH";
>  static const char *fmt_pretty;
> +static const char *log_merges;

The variable name may want to be updated to mimic other variables
used in a similar way, e.g. default_show_root is used to store the
value from log.showroot.

Perhaps "default_show_merge" or something?

Thanks.

>  static const char * const builtin_log_usage[] = {
>  	N_("git log [<options>] [<revision range>] [[--] <path>...]"),
> @@ -386,6 +387,9 @@ static int git_log_config(const char *var, const char *value, void *cb)
>  			decoration_style = 0; /* maybe warn? */
>  		return 0;
>  	}
> +	if (!strcmp(var, "log.merges")) {
> +		return git_config_string(&log_merges, var, value);
> +	}
>  	if (!strcmp(var, "log.showroot")) {
>  		default_show_root = git_config_bool(var, value);
>  		return 0;
> @@ -628,6 +632,8 @@ int cmd_log(int argc, const char **argv, const char *prefix)
>  
>  	init_revisions(&rev, prefix);
>  	rev.always_show_header = 1;
> +	if (log_merges && parse_merges_opt(&rev, log_merges))
> +		die("unknown config value for log.merges: %s", log_merges);
>  	memset(&opt, 0, sizeof(opt));
>  	opt.def = "HEAD";
>  	opt.revarg_opt = REVARG_COMMITTISH;

next prev parent reply	other threads:[~2015-04-04 20:00 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <266077>
2015-04-04  1:21 ` [PATCH v2 1/5] revision: add --merges={show|only|hide} option Koosha Khajehmoogahi
2015-04-04  1:21   ` [PATCH v2 2/5] log: honor log.merges= option Koosha Khajehmoogahi
2015-04-04 20:00     ` Junio C Hamano [this message]
2015-04-07 22:15       ` Koosha Khajehmoogahi
2015-04-08  2:28         ` Junio C Hamano
2015-04-08 10:42           ` Koosha Khajehmoogahi
2015-04-13  4:56             ` Junio C Hamano
2015-04-07  5:18     ` Eric Sunshine
2015-04-04  1:21   ` [PATCH v2 3/5] Documentation: add git-log --merges= option and log.merges config. var Koosha Khajehmoogahi
2015-04-05 21:41     ` Junio C Hamano
2015-04-04  1:22   ` [PATCH v2 4/5] t4202-log: add tests for --merges= Koosha Khajehmoogahi
2015-04-07  7:32     ` Eric Sunshine
2015-04-04  1:22   ` [PATCH v2 5/5] bash-completion: add support for git-log --merges= and log.merges Koosha Khajehmoogahi
2015-04-07  5:16   ` [PATCH v2 1/5] revision: add --merges={show|only|hide} option Eric Sunshine

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=xmqqy4m7ek9q.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=koosha@posteo$(echo .)de \
    --cc=sunshine@sunshineco$(echo .)com \
    /path/to/YOUR_REPLY

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox