From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.6 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 454AB1F55B for ; Thu, 4 Jun 2020 19:27:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729200AbgFDT0e (ORCPT ); Thu, 4 Jun 2020 15:26:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42968 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729184AbgFDT0d (ORCPT ); Thu, 4 Jun 2020 15:26:33 -0400 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6F637C08C5C0 for ; Thu, 4 Jun 2020 12:26:33 -0700 (PDT) Received: by mail-pj1-x1043.google.com with SMTP id jz3so1567286pjb.0 for ; Thu, 04 Jun 2020 12:26:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=5+BAaBQeC+LS8Ezx0amMTrBULCiOFzFLTR0YXxKu4+c=; b=ltKgRv30I/M5Sc/lazZuVwxvsZ81ALE0179gpillTN9GM7185xW/8aOiD2xy5TbSiL e8tHY9X0jpsF6gCwqvzYBePwcPKljG0CgPRBilVAxkYfwQ1kr0j/i7kNNATJnr0L2LiA GDt+WfN4AocxVIbtdZSnXcls7wDPzArv1drUw77Z2zf4cAamdUqFcSoSFbJ9i/DuIkGZ y+bd8uWxFWp0IrjSahQ8PPV3pdDu8Kkvqe08H1zqt+IuKgFe3wgsVJBP9l//RckgFzkc Ou663GotLYz1bwICqLbP7mJI8grJE+GjYXRsYlob66kPKmwwb1CT8Mkv2x/brosbwy9k R3mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=5+BAaBQeC+LS8Ezx0amMTrBULCiOFzFLTR0YXxKu4+c=; b=dRwwzLII6nolJ4Xwbzb9K3NXUiloGCRQ029XC16VnGDHY3b97BplBrgzkbmUy3gRj0 rT1qd4fieo2zXDNzB9XnbwM5BSuhSEArIC4612S83xi0rB2+i0kspNqTGFv09XZMJ+vV QUPiR3httqkxJaLAZjHkc2X6SwPTh5D2XbzFv+ZDd2CgRHU5W/xQrL8/SH3kBPZs5zde nxdoMAXjVo/nbMDZIWW8ov5eA0UXnrywTI4g9TZ3SvyjJNAuXvw82XrI6ZpJOOAHjHi3 48flyekkrHkAls0STV7+Ag2KbCh5n5VSc6rCUNJXD67oCTl/9lxW48dz0ybY+u6Yqcuy QwCw== X-Gm-Message-State: AOAM530ftDTcU2f660HK6SbZv6bH6K2/Q15yOOp4Y8L1mJT7e3ldshxn amplD35kaUdNtdpl/S8L/NCEH7q9 X-Google-Smtp-Source: ABdhPJxISEVP6AZG1GU2/DjCykm4PsCllyeLyiVn9omFJ7ffyzg49/zhCag3XZNn3qry7Qe/kzgfVA== X-Received: by 2002:a17:902:9686:: with SMTP id n6mr6137844plp.186.1591298792941; Thu, 04 Jun 2020 12:26:32 -0700 (PDT) Received: from [192.168.208.37] ([49.207.141.8]) by smtp.gmail.com with ESMTPSA id b16sm5121729pfd.111.2020.06.04.12.26.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 04 Jun 2020 12:26:32 -0700 (PDT) Subject: Re: [GSoC][PATCH v5] submodule: port subcommand 'set-branch' from shell to C To: Christian Couder Cc: git , Shourya Shukla , Johannes Schindelin , Christian Couder , =?UTF-8?B?xJBvw6BuIFRy4bqnbiBDw7RuZyBEYW5o?= , Junio C Hamano , Denton Liu , Eric Sunshine References: <20200523163929.7040-1-shouryashukla.oo@gmail.com> <20200602163523.7131-1-shouryashukla.oo@gmail.com> <1b851e49-3bb1-3b59-7f24-b903c5514391@gmail.com> From: Kaartic Sivaraam Message-ID: <92bad281-dd38-aef2-9910-659b41cdd830@gmail.com> Date: Fri, 5 Jun 2020 00:56:20 +0530 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 03-06-2020 01:15, Christian Couder wrote: > On Tue, Jun 2, 2020 at 9:01 PM Kaartic Sivaraam > wrote: >> >> I'm having second thoughts about my suggestion[1] to include >> the short option for '--quiet' in the usage. This is the only >> usage in submodule--helper that mentions that '-q' is a short >> hand for '--quiet'. That seems inconsistent. I see two ways but >> I'm not sure which one of these would be better: >> >> A. Dropping the mention of '-q' in this usage thus making it consistent >> with the other usages printed by submodule--helper. >> >> B. Fixing other usages of submodule--helper to mention that '-q' is >> shorthand for quiet. This has the benefit of properly advertising >> the shorthand. >> >> C. Just ignore this? > > The `git submodule` documentation has: > > -q:: > --quiet:: > Only print error messages. > > even though the Synopsis is: > > 'git submodule' [--quiet] [--cached] > 'git submodule' [--quiet] add [] [--] [] > 'git submodule' [--quiet] status [--cached] [--recursive] [--] [...] > ... > > So I prefer B, and maybe updating the synopsis, as I think most Git > commands have '-q' meaning '--quiet'. > Makes sense. > [...] > >> So, according to this, I think the usage should be ... >> >> git submodule--helper set-branch [-q | --quiet] [-d | --default] >> >> ... and ... >> >> git submodule--helper set-branch [-q|--quiet] [-b | --branch] >> >> ... respectively. > > I don't agree. I think `git submodule--helper set-branch ...` requires > either "-d | --default" or "-b | --branch", while for example: > > git submodule--helper set-branch [-q | --quiet] [-d | --default] > > would mean that "git submodule--helper set-branch my/path" is valid. > You're right. Even I thought about the same thing when I came up with that suggestion after quoting that portion of the CodingGuidelines. But it was also curious for me to observe that the original used parenthesis to mention the short and long options of an argument: > + const char *const usage[] = { > + N_("git submodule--helper set-branch [-q|--quiet] (-d|--default) "), > + N_("git submodule--helper set-branch [-q|--quiet] (-b|--branch) "), > + NULL > + }; I've not seen such a usage before. That's what actually made me take a look at the CodingGuidelines for this. As the CodingGuidelined doesn't seem to be mentioning anything about this explicitly, let's see if I could find something in the usage printed by other commands. --- > git am -h usage: git am [] [( | )...] or: git am [] (--continue | --skip | --abort) > git branch -h usage: git branch [] [-r | -a] [--merged | --no-merged] or: git branch [] [-l] [-f] [] or: git branch [] [-r] (-d | -D) ... or: git branch [] (-m | -M) [] or: git branch [] (-c | -C) [] or: git branch [] [-r | -a] [--points-at] or: git branch [] [-r | -a] [--format] > git checkout -h usage: git checkout [] or: git checkout [] [] -- ... > git switch -h usage: git switch [] [] --- Hmm. Looks like it's not common for us to mention both the short and long options in the usage itself. This might be to avoid the redundancy as the usage is usually followed by the list of options. With this info, I think we could've just gone with the following as the usage strings for the `set-branch` subcommand: git submodule--helper set-branch [] -d git submodule--helper set-branch [] -b This also solves the problem with `--quiet` I mentioned earlier while making it concise and inline with the usages printed by other commands. All this said, I don't think it's worth a re-roll now for several reasons. -- Sivaraam