All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: "Git List" <git@vger.kernel.org>, "Jeff King" <peff@peff.net>,
	"Junio C Hamano" <gitster@pobox.com>,
	"René Scharfe" <l.s.r@web.de>
Subject: Re: [PATCH v3 1/2] builtin/config.c: treat type specifiers singularly
Date: Wed, 4 Apr 2018 03:57:12 -0400	[thread overview]
Message-ID: <CAPig+cRNSYz_guBS9yNEXdAbfG+FWtvnzwsWRu0DRe0N_VkiOQ@mail.gmail.com> (raw)
In-Reply-To: <20180404060743.39278-2-me@ttaylorr.com>

On Wed, Apr 4, 2018 at 2:07 AM, Taylor Blau <me@ttaylorr.com> wrote:
> [...]
> In fact, `git config` will not accept multiple type specifiers at a
> time, as indicated by:
>
>   $ git config --int --bool some.section
>   error: only one type at a time.
>
> This patch uses `OPT_SET_INT` to prefer the _last_ mentioned type
> specifier, so that the above command would instead be valid, and a
> synonym of:
>
>   $ git config --bool some.section
>
> This change is motivated by two urges: (1) it does not make sense to
> represent a singular type specifier internally as a bitset, only to
> complain when there are multiple bits in the set. `OPT_SET_INT` is more
> well-suited to this task than `OPT_BIT` is. (2) a future patch will
> introduce `--type=<type>`, and we would like not to complain in the
> following situation:
>
>   $ git config --int --type=int

I can understand "last wins" for related options such as "--verbose
--quiet" or "--verbose=1 --verbose=2", but I have a lot of trouble
buying the "last wins" argument in this context where there is no
semantic relationship between, say, --bool and --expiry-date.
Specifying conflicting options together is likely to be unintentional,
thus reporting it as an error seems sensible, so I'm not convinced
that patch's removal of that error is a good idea.

I understand that you're doing this to avoid complaining about "--int
--type=int", but exactly how that case is supported should be an
implementation detail; it doesn't need to bleed into the UI as an
unnecessary and not-well-justified loosening of an age-old
restriction. There are other ways to support "--int --type=int"
without "last wins". Also, do we really need to support "--int
--type=int"? Is that an expected use-case?

  reply	other threads:[~2018-04-04  7:57 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-28 23:47 [PATCH] builtin/config.c: prefer `--type=bool` over `--bool`, etc Taylor Blau
2018-03-29 20:18 ` Junio C Hamano
2018-03-29 22:11 ` Jeff King
2018-03-30  5:27   ` Taylor Blau
2018-03-30 13:53     ` Jeff King
2018-03-30 16:00       ` Junio C Hamano
2018-03-30 18:27         ` Jeff King
2018-03-30  5:28 ` [PATCH v2 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-03-30  5:28   ` [PATCH v2 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc Taylor Blau
2018-03-30  6:17     ` René Scharfe
2018-03-30 13:48     ` Jeff King
2018-03-30 13:41   ` [PATCH v2 1/2] builtin/config.c: treat type specifiers singularly Jeff King
2018-04-04  6:07 ` [PATCH v3 0/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc Taylor Blau
2018-04-04  6:07   ` [PATCH v3 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-04  7:57     ` Eric Sunshine [this message]
2018-04-05  1:53       ` Taylor Blau
2018-04-05 21:51         ` Jeff King
2018-04-04  6:07   ` [PATCH v3 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc Taylor Blau
2018-04-04  7:27     ` Eric Sunshine
2018-04-05  1:47       ` Taylor Blau
2018-04-05  2:00 ` [PATCH v4 0/2] " Taylor Blau
2018-04-05 21:58   ` Jeff King
2018-04-05 22:15     ` Taylor Blau
     [not found] ` <cover.1522893363.git.me@ttaylorr.com>
2018-04-05  2:00   ` [PATCH v4 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-05  2:00   ` [PATCH v4 2/2] builtin/config.c: prefer `--type=bool` over `--bool`, etc Taylor Blau
2018-04-05 22:29     ` Eric Sunshine
2018-04-05 22:40       ` Jeff King
2018-04-06  5:19         ` Taylor Blau
2018-04-06  5:17       ` Taylor Blau
2018-04-05  2:02   ` Taylor Blau
2018-04-05 22:12     ` Jeff King
2018-04-05 22:15       ` Taylor Blau
2018-04-06  5:08       ` Taylor Blau
2018-04-06  6:38 ` [PATCH v6 0/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type` Taylor Blau
     [not found] ` <cover.1522996619.git.me@ttaylorr.com>
2018-04-06  6:39   ` [PATCH v6 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-06  6:39   ` [PATCH v6 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type` Taylor Blau
2018-04-06  7:04     ` Eric Sunshine
2018-04-07  0:39       ` Taylor Blau
2018-04-07  8:25         ` Eric Sunshine
2018-04-09 22:46 ` [PATCH v7 0/2] " Taylor Blau
2018-04-09 23:11   ` Eric Sunshine
     [not found] ` <cover.1523313730.git.me@ttaylorr.com>
2018-04-09 22:46   ` [PATCH v7 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-10  1:22     ` Junio C Hamano
2018-04-10  2:12       ` Taylor Blau
2018-04-10  4:13         ` Eric Sunshine
2018-04-09 22:46   ` [PATCH v7 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type` Taylor Blau
2018-04-10  1:44     ` Junio C Hamano
2018-04-10  2:17       ` Taylor Blau
2018-04-11  1:06 ` [PATCH v8 0/2] " Taylor Blau
2018-04-11  1:24   ` Junio C Hamano
2018-04-11  1:33     ` Taylor Blau
2018-04-11  3:11       ` Junio C Hamano
2018-04-11  3:49         ` Taylor Blau
     [not found] ` <cover.1523408336.git.me@ttaylorr.com>
2018-04-11  1:06   ` [PATCH v8 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-11  1:07   ` [PATCH v8 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type` Taylor Blau
2018-04-18 21:43 ` [PATCH v9 0/2] " Taylor Blau
     [not found] ` <cover.1524087557.git.me@ttaylorr.com>
2018-04-18 21:43   ` [PATCH v9 1/2] builtin/config.c: treat type specifiers singularly Taylor Blau
2018-04-18 21:43   ` [PATCH v9 2/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type` Taylor Blau
2018-04-19  2:47     ` Junio C Hamano
2018-04-19  3:01       ` Taylor Blau

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=CAPig+cRNSYz_guBS9yNEXdAbfG+FWtvnzwsWRu0DRe0N_VkiOQ@mail.gmail.com \
    --to=sunshine@sunshineco.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.