git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "SZEDER Gábor" <szeder@ira.uka.de>
To: Pranit Bauva <pranit.bauva@gmail.com>
Cc: "SZEDER Gábor" <szeder@ira.uka.de>, git@vger.kernel.org
Subject: [PATCH v9 1/3] parse-options.c: make OPTION__COUNTUP consider negative values
Date: Thu, 24 Mar 2016 11:33:13 +0100	[thread overview]
Message-ID: <1458815593-11060-1-git-send-email-szeder@ira.uka.de> (raw)
In-Reply-To: <01020153a7ba4e07-f326395b-63df-4f8a-b378-8f387b02d0bd-000000@eu-west-1.amazonses.com>

> The reason to make it consider negative values or more specifically
> "unspecified" values is to give the ability to differentiate between
> once, multiple time or with --no-option.
> 
> Eg. :
> initialize verbose = -1
> `git commit` => verbose = -1
> `git commit -v` => verbose = 1
> `git commit -v -v` => verbose = 2
> `git commit --no-verbose` => verbose = 0
> 
> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com>
> 
> ---
> The discussion about this patch:
> [1] : http://thread.gmane.org/gmane.comp.version-control.git/289027
> 
> Previous version of the patch:
> [v1] : http://thread.gmane.org/gmane.comp.version-control.git/289061
> 
> Changes introduced:
> Use a different language in commit message to make the change and its
> utility more clear.

I don't see the mentioned change in the commit message.  In
particular:

  - As Eric pointed out in the previous round, the commit message
    misses the single most important point of justification: to
    determine whether '--option' or '--no-option' was specified at
    all.  Explaining this would also make the examples unnecessary.

  - OPT_COUNTUP is used in several places, mostly indirecty, but the
    commit message doesn't explain that possible side-effects to these
    callers were considered and that they are not harmed by this
    change.

> ---
>  Documentation/technical/api-parse-options.txt | 6 ++++--
>  parse-options.c                               | 8 +++++++-
>  2 files changed, 11 insertions(+), 3 deletions(-)

A couple of new tests to t0040-parse-options.sh would be great to
ensure that starting from a negative value works as advertised, i.e.
at least that '--option' jumps to 1 and '--no-option' resets to 0.

> diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
> index 5f0757d..a908d8a 100644
> --- a/Documentation/technical/api-parse-options.txt
> +++ b/Documentation/technical/api-parse-options.txt
> @@ -144,8 +144,10 @@ There are some macros to easily define options:
>  
>  `OPT_COUNTUP(short, long, &int_var, description)`::
>  	Introduce a count-up option.
> -	`int_var` is incremented on each use of `--option`, and
> -	reset to zero with `--no-option`.
> +	If the `int_var` is negative and `--option` is absent,
> +	then it will retain its value. Otherwise it will first set
> +	`int_var` to 0 and then increment it on each use of `--option`,
> +	and reset to zero with `--no-option`.
>  
>  `OPT_BIT(short, long, &int_var, description, mask)`::
>  	Introduce a boolean option.
> diff --git a/parse-options.c b/parse-options.c
> index 47a9192..86d30bd 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -110,7 +110,13 @@ static int get_value(struct parse_opt_ctx_t *p,
>  		return 0;
>  
>  	case OPTION_COUNTUP:
> -		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
> +		if (unset)
> +			*(int *)opt->value = 0;
> +		else {
> +			if (*(int *)opt->value < 0)
> +				*(int *)opt->value = 0;
> +			*(int *)opt->value += 1;
> +		}
>  		return 0;
>  
>  	case OPTION_SET_INT:
> 
> --
> https://github.com/git/git/pull/218
> 
> 

  parent reply	other threads:[~2016-03-24 10:33 UTC|newest]

Thread overview: 136+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-14 21:38 [PATCH v7] commit: add a commit.verbose config variable Pranit Bauva
2016-03-15 11:31 ` SZEDER Gábor
2016-03-15 19:00   ` Pranit Bauva
2016-03-15 19:24     ` Eric Sunshine
2016-03-15 20:13       ` Pranit Bauva
2016-03-15 20:24         ` Junio C Hamano
2016-03-15 21:09           ` Pranit Bauva
2016-03-15 21:16             ` Junio C Hamano
2016-03-15 21:18               ` Pranit Bauva
2016-03-18 21:19 ` [PATCH v8 1/2] parse-options.c: make OPTION__COUNTUP consider negative values Pranit Bauva
2016-03-18 21:19   ` [PATCH v8 2/2] commit: add a commit.verbose config variable Pranit Bauva
2016-03-20  3:56     ` Eric Sunshine
2016-03-20 11:05       ` Pranit Bauva
2016-03-20 17:34         ` Eric Sunshine
2016-03-20 18:02           ` Pranit Bauva
2016-03-23 19:19             ` Junio C Hamano
2016-03-23 19:23               ` Pranit Bauva
2016-03-23 20:43                 ` Junio C Hamano
2016-03-24  8:25     ` [PATCH v9 1/3] parse-options.c: make OPTION__COUNTUP consider negative values Pranit Bauva
2016-03-24  8:25       ` [PATCH v9 2/3] t7507-commit-verbose: make test suite use write_script Pranit Bauva
2016-03-24 11:00         ` SZEDER Gábor
2016-03-24 23:57           ` Eric Sunshine
2016-03-25  6:06             ` Pranit Bauva
2016-03-25  6:24               ` Eric Sunshine
2016-03-25  6:55                 ` Pranit Bauva
2016-03-25 14:46             ` SZEDER Gábor
2016-03-25 14:50               ` Pranit Bauva
2016-03-25 17:04               ` Eric Sunshine
2016-03-25 18:15                 ` Pranit Bauva
2016-03-25 23:06                   ` Eric Sunshine
2016-03-24  8:25       ` [PATCH v9 3/3] commit: add a commit.verbose config variable Pranit Bauva
2016-03-24 10:04         ` SZEDER Gábor
2016-03-24 10:22           ` Pranit Bauva
2016-03-24 10:26             ` Pranit Bauva
2016-03-25  0:05         ` Eric Sunshine
2016-03-25  6:15           ` Pranit Bauva
2016-03-24 10:33       ` SZEDER Gábor [this message]
2016-03-24 10:38         ` [PATCH v9 1/3] parse-options.c: make OPTION__COUNTUP consider negative values Pranit Bauva
2016-03-24 23:34         ` Eric Sunshine
2016-03-28 18:42         ` Pranit Bauva
2016-03-28 19:03           ` Eric Sunshine
2016-03-26 19:48       ` [PATCH v10 1/3] parse-options.c: make OPTION__COUNTUP understand "unspecified" values Pranit Bauva
2016-03-26 19:48         ` [PATCH v10 2/3] t7507-commit-verbose: store output of grep in a file Pranit Bauva
2016-03-27  3:07           ` Eric Sunshine
2016-03-27 13:27             ` SZEDER Gábor
2016-03-27 13:43               ` Pranit Bauva
2016-03-27 17:27               ` Eric Sunshine
2016-03-27 18:31                 ` Pranit Bauva
     [not found]             ` <CAFZEwPMaZkUi+DvohhVrc_dW_8cdfJsZX-YA_SRWDp021UvDLQ@mail.gmail.com>
2016-03-27 17:03               ` Eric Sunshine
     [not found]               ` <CAPig+cTFK=HPAtk7MeMQSTccmiaai++3sVn6J_pRcSi+w_4Lng@mail.gmail.com>
2016-03-27 17:05                 ` Eric Sunshine
     [not found]                 ` <CAFZEwPMJiCTKszfCAVrzsA+jNHwoHPaXySSD3HyiO=f5AikvZg@mail.gmail.com>
     [not found]                   ` <CAPig+cS3usDDeTMzjqbxqi+k+XbmjawZ0TF20s9-vM6o=Fm=OQ@mail.gmail.com>
2016-03-27 17:08                     ` Eric Sunshine
2016-03-26 19:48         ` [PATCH v10 3/3] commit: add a commit.verbose config variable Pranit Bauva
2016-03-27  3:34           ` Eric Sunshine
2016-03-27  7:00             ` Pranit Bauva
2016-03-27  8:17               ` Eric Sunshine
2016-03-27  8:40                 ` Pranit Bauva
2016-03-27 11:51           ` SZEDER Gábor
2016-03-27 11:59             ` Pranit Bauva
2016-03-27 12:07               ` SZEDER Gábor
2016-03-27 12:11                 ` Pranit Bauva
2016-03-27 16:48               ` Eric Sunshine
2016-03-27  2:45         ` [PATCH v10 1/3] parse-options.c: make OPTION__COUNTUP understand "unspecified" values Eric Sunshine
2016-03-27  6:10           ` Pranit Bauva
2016-03-31 14:45         ` [PATCH v11 1/4] test-parse-options: print quiet as integer Pranit Bauva
2016-03-31 14:45           ` [PATCH v11 4/4] commit: add a commit.verbose config variable Pranit Bauva
2016-03-31 14:45           ` [PATCH v11 2/4] parse-options.c: make OPTION_COUNTUP respect "unspecified" values Pranit Bauva
2016-03-31 18:41             ` Junio C Hamano
2016-03-31 19:34               ` Pranit Bauva
2016-03-31 20:06                 ` Junio C Hamano
2016-03-31 20:41                   ` Pranit Bauva
2016-03-31 20:45                     ` Junio C Hamano
2016-03-31 14:45           ` [PATCH v11 3/4] t7507-commit-verbose: improve test coverage by testing number of diffs Pranit Bauva
2016-03-31 18:23             ` Junio C Hamano
2016-03-31 18:42               ` Pranit Bauva
2016-03-31 18:19           ` [PATCH v11 1/4] test-parse-options: print quiet as integer Junio C Hamano
2016-03-31 18:40             ` Pranit Bauva
2016-04-02 23:33           ` [PATCH v12 1/5] t0040-test-parse-options.sh: fix style issues Pranit Bauva
2016-04-02 23:33             ` [PATCH v12 2/5] test-parse-options: print quiet as integer Pranit Bauva
2016-04-03 21:30               ` Eric Sunshine
2016-04-05 15:39                 ` Pranit Bauva
2016-04-06  5:56                   ` Eric Sunshine
2016-04-08 11:33                   ` Duy Nguyen
2016-04-08 16:52                 ` Junio C Hamano
2016-04-02 23:33             ` [PATCH v12 4/5] t7507-commit-verbose: improve test coverage by testing number of diffs Pranit Bauva
2016-04-04  0:02               ` Eric Sunshine
2016-04-04  1:05                 ` Eric Sunshine
2016-04-05 15:54                 ` Pranit Bauva
2016-04-02 23:33             ` [PATCH v12 3/5] parse-options.c: make OPTION_COUNTUP respect "unspecified" values Pranit Bauva
2016-04-03 23:10               ` Eric Sunshine
2016-04-05 15:51                 ` Pranit Bauva
2016-04-02 23:33             ` [PATCH v12 5/5] commit: add a commit.verbose config variable Pranit Bauva
2016-04-04  0:58               ` Eric Sunshine
2016-04-04 23:29                 ` Eric Sunshine
2016-04-05 15:58                   ` Pranit Bauva
2016-04-03 21:00             ` [PATCH v12 1/5] t0040-test-parse-options.sh: fix style issues Eric Sunshine
2016-04-04 12:45               ` Pranit Bauva
2016-04-04 17:30                 ` Eric Sunshine
2016-04-05  5:08                   ` Pranit Bauva
2016-04-09 12:23             ` [PATCH v13 1/6] " Pranit Bauva
2016-04-09 12:23               ` [PATCH v13 5/6] t7507-commit-verbose: improve test coverage by testing number of diffs Pranit Bauva
2016-04-09 12:23               ` [PATCH v13 3/6] t0040-parse-options: improve test coverage Pranit Bauva
2016-04-09 12:23               ` [PATCH v13 4/6] parse-options.c: make OPTION_COUNTUP respect "unspecified" values Pranit Bauva
2016-04-09 12:23               ` [PATCH v13 2/6] test-parse-options: print quiet as integer Pranit Bauva
2016-04-12 21:33                 ` Junio C Hamano
2016-04-12 22:16                   ` Pranit Bauva
2016-04-12 23:11                     ` Junio C Hamano
2016-04-09 12:23               ` [PATCH v13 6/6] commit: add a commit.verbose config variable Pranit Bauva
2016-04-12 21:24                 ` Junio C Hamano
2016-04-12 21:28                   ` Pranit Bauva
2016-04-12 21:39                   ` Junio C Hamano
2016-04-12 22:18                     ` Junio C Hamano
2016-04-12 22:25                       ` Pranit Bauva
2016-04-12 23:02               ` [PATCH v14 1/6] t0040-test-parse-options.sh: fix style issues Pranit Bauva
2016-04-12 23:02                 ` [PATCH v14 4/6] parse-options.c: make OPTION_COUNTUP respect "unspecified" values Pranit Bauva
2016-04-13  5:56                   ` Eric Sunshine
2016-04-13  8:39                     ` Pranit Bauva
2016-04-13 17:33                       ` Eric Sunshine
2016-04-13 17:43                         ` Pranit Bauva
2016-04-13 17:50                           ` Eric Sunshine
2016-04-12 23:02                 ` [PATCH v14 3/6] t0040-parse-options: improve test coverage Pranit Bauva
2016-04-13  5:26                   ` Eric Sunshine
2016-04-13  8:59                     ` Pranit Bauva
2016-04-13 17:27                       ` Eric Sunshine
2016-04-25 18:40                         ` Pranit Bauva
2016-04-27 17:55                           ` Eric Sunshine
2016-04-27 18:16                             ` Pranit Bauva
2016-04-12 23:02                 ` [PATCH v14 5/6] t7507-commit-verbose: improve test coverage by testing number of diffs Pranit Bauva
2016-04-13  6:03                   ` Eric Sunshine
2016-04-13  9:00                     ` Pranit Bauva
2016-04-12 23:02                 ` [PATCH v14 6/6] commit: add a commit.verbose config variable Pranit Bauva
2016-04-13  6:14                   ` Eric Sunshine
2016-04-13  9:15                     ` Pranit Bauva
2016-04-13 17:44                       ` Eric Sunshine
2016-04-12 23:02                 ` [PATCH v14 2/6] test-parse-options: print quiet as integer Pranit Bauva
2016-03-18 22:59   ` [PATCH v8 1/2] parse-options.c: make OPTION__COUNTUP consider negative values Junio C Hamano
2016-03-19  4:55     ` Pranit Bauva

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=1458815593-11060-1-git-send-email-szeder@ira.uka.de \
    --to=szeder@ira.uka.de \
    --cc=git@vger.kernel.org \
    --cc=pranit.bauva@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).