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: AS3215 2.6.0.0/16 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, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from out1.vger.email (out1.vger.email [IPv6:2620:137:e000::1:20]) by dcvr.yhbt.net (Postfix) with ESMTP id 1C4671F4D7 for ; Thu, 16 Jun 2022 22:37:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S245332AbiFPWhc (ORCPT ); Thu, 16 Jun 2022 18:37:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:43730 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231899AbiFPWha (ORCPT ); Thu, 16 Jun 2022 18:37:30 -0400 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A8431AE for ; Thu, 16 Jun 2022 15:37:28 -0700 (PDT) Received: (qmail 15960 invoked by uid 109); 16 Jun 2022 22:37:28 -0000 Received: from Unknown (HELO sigill.intra.peff.net) (10.0.0.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 16 Jun 2022 22:37:28 +0000 Authentication-Results: cloud.peff.net; auth=none Date: Thu, 16 Jun 2022 18:37:27 -0400 From: Jeff King To: Brad Forschinger via GitGitGadget Cc: git@vger.kernel.org, Brad Forschinger Subject: Re: [PATCH] git-prompt: use builtin test Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Tue, Jun 14, 2022 at 09:09:11AM +0000, Brad Forschinger via GitGitGadget wrote: > From: Brad Forschinger > > The test and [ commands are used throughout the prompt generation. They > also happen to be valid function names that can be defined, leading to > unintentional results. Prevent the somewhat unusual case of this > happening by simply using [[, which is reserved. Hmm. I do think we need to be a bit more paranoid about style in the prompt and completion code, because they are sourced into the user's shell alongside whatever other weird customizations they'd have. So we already have adjustments to work under "set -u", and so forth. But at some point we may say "you have made the environment too hostile for us to function". Is redefining "test" to something that doesn't behave the same way such a case? Part of me wants to say yes. :) That said, if it's not _hard_ to support, maybe it is worth doing to be on the cautious side? A few thoughts: - my biggest concern on cost is that this is an unusual style for our project (which usually writes in POSIX shell, though of course this file is meant to be bash/zsh specific). Will it be a maintenance burden going forward? - this only changes git-prompt.sh; doesn't the completion code have the same issue? - I don't write much bash-specific code, but I seem to recall that "[[" has some subtle differences to "[". Is it sufficiently a superset that these conversions are all equivalent? I think some like: > - if [ $pcmode = yes ] && [ $ps1_expanded = yes ]; then > + if [[ $pcmode = yes ]] && [[ $ps1_expanded = yes ]]; then are not equivalent, but it's an actual improvement (bash's builtin "[[" isn't confused by unquoted empty variables), but I don't know if there may be other gotchas. (I doubt this is an actual bug in the current code, as $pcmode always seems to be set, but just a more defensive style). -Peff