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-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.9 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id E191B1F463 for ; Thu, 19 Dec 2019 18:03:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726971AbfLSSDZ (ORCPT ); Thu, 19 Dec 2019 13:03:25 -0500 Received: from smtprelay01.ispgateway.de ([80.67.18.43]:25127 "EHLO smtprelay01.ispgateway.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726880AbfLSSDY (ORCPT ); Thu, 19 Dec 2019 13:03:24 -0500 Received: from [24.134.116.61] (helo=[192.168.92.208]) by smtprelay01.ispgateway.de with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.92.3) (envelope-from ) id 1ii08x-0002HI-OH; Thu, 19 Dec 2019 19:03:23 +0100 Subject: Re: [PATCH 2/5] parse_branchname_arg(): introduce expect_commit_only To: Junio C Hamano , Alexandr Miloslavskiy via GitGitGadget Cc: git@vger.kernel.org References: From: Alexandr Miloslavskiy Message-ID: <713f8aea-cdd6-3fcd-8e9f-c9b909454ef5@syntevo.com> Date: Thu, 19 Dec 2019 19:03:23 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Df-Sender: YWxleGFuZHIubWlsb3NsYXZza2l5QHN5bnRldm8uY29t Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On 18.12.2019 20:18, Junio C Hamano wrote: >> `has_dash_dash` unexpectedly takes `opts->accept_pathspec` into account. > > You also touched the code that depends on opts->accept_pathspec in > the earlier step 1/5; these two steps seem harder to reason about > than necessary---I wonder if it is easier to explain and understand > if these two steps are merged into one and explained together? Yes, that sounds like a good idea! In V3 in other topic [1] I have shuffled the changes between commits for easier understanding. >> + if (has_dash_dash) >> + expect_commit_only = 1; > > Non-standard indentation here. Sorry! >> + opts->count_checkout_paths = !opts->quiet && !expect_commit_only; > > OK. count_checkout_paths is relevant only when checking out paths > out of a tree-ish, so "expect-commit-only" would be false in such a > situation. On the other hand, if we were checking out a branch (or > detaching), we must have a commit and a tree-ish is insufficient, > so expect_commit_only would be true in such a case. > > Makes sense. I am wondering if we still need has_dash_dash, and > also if expect_commit_only is the best name for the variable. It seems that indeed, the variable could use an even better name. It's as opposed to , and not as opposed to . I have renamed the variable again in V3 in other topic [1]. ---- [1] https://lore.kernel.org/git/pull.490.v3.git.1576778515.gitgitgadget@gmail.com/