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.8 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 2D73C1F403 for ; Sat, 8 Oct 2022 17:47:05 +0000 (UTC) Authentication-Results: dcvr.yhbt.net; dkim=pass (1024-bit key; unprotected) header.d=pobox.com header.i=@pobox.com header.b="GGgr+Ipv"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230260AbiJHRrA (ORCPT ); Sat, 8 Oct 2022 13:47:00 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42184 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229724AbiJHRq6 (ORCPT ); Sat, 8 Oct 2022 13:46:58 -0400 Received: from pb-smtp2.pobox.com (pb-smtp2.pobox.com [64.147.108.71]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id B35A53498A for ; Sat, 8 Oct 2022 10:46:57 -0700 (PDT) Received: from pb-smtp2.pobox.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id A4ED514E149; Sat, 8 Oct 2022 13:46:56 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type; s=sasl; bh=vfK3hjV+lP4ev+L0NuysvYcN1aBUF5wvYvTwsz wTAqg=; b=GGgr+IpvyXwcd6YUMEesMU/9l44gUBICHxhGzRC7VWL63Gv9madVcB WD7LwdI2DH8QB82wnLfjAspoEfeLMpZxbnMwIpDZBbjpLglTtT5oaw08fNnVWEzm 6CAp+w2FF4bPvViTT+Ymhb/8Pw69j70oM0mjS5Ssgj6Qe7uPtvPN4= Received: from pb-smtp2.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp2.pobox.com (Postfix) with ESMTP id 9D55514E148; Sat, 8 Oct 2022 13:46:56 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.83.5.33]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp2.pobox.com (Postfix) with ESMTPSA id 0A42814E147; Sat, 8 Oct 2022 13:46:55 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: Eric Sunshine Cc: =?utf-8?Q?Rub=C3=A9n?= Justo , Git List Subject: Re: [PATCH v4] branch: support for shortcuts like @{-1}, completed References: <93b0b442-b277-66a6-3f5f-5a498593aa07@gmail.com> <7abdb5a9-5707-7897-4196-8d2892beeb81@gmail.com> <2e164aea-7dd8-5018-474a-01643553ea49@gmail.com> Date: Sat, 08 Oct 2022 10:46:54 -0700 In-Reply-To: (Eric Sunshine's message of "Sat, 8 Oct 2022 13:10:59 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Pobox-Relay-ID: 337446C8-4731-11ED-ACCA-307A8E0A682E-77302942!pb-smtp2.pobox.com Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Eric Sunshine writes: >> Yeah, I thought about that. What convinced me to use "git stripspace" was >> that maybe that '\n' tail could be removed sometime from the description >> setting and this will be fine with that. I haven't found any reason for >> that '\n' and it bugs me a little seeing it in the config :-) > > That reasoning occurred to me, as well, and I'd have no objection to > git-stripspace if that's the motivation for using it. I don't feel > strongly one way or the other, and my previous email was intended > primarily to point out the lightweight alternatives in case you hadn't > considered them. Feel free to use git-stripspace if you feel it is the > more appropriate choice. I do not think I would agree with the line of reasoning. It all depends on why we anticipate that the terminating LF may go away someday, but if it is because we may do so by mistake and without a good reason when making some unrelated changes to the implementation of "git branch --edit-desc", we would want to know about it, and such a loose check that would miss it is definitely unwelcome. It is very likely that not just "git merge" but people's external scripts depend on the presence of final LF especially when the description has only one line, so unless we are doing deliberately so, we should prepare to catch such a change. If it is because we may gain a consensus that the description string (which by the way can well consist of multiple lines) is better without the LF on its final line, and we are "fixing" the code to do so very much on purpose, it would be good to have a test to protect such a change from future unintended breakages. Adding a loose test that won't break across such a change today may be OK, but whoever is making such a change in the future has to make sure there is a test that is not loose to protect the change. And it would very likely to be done by adding a new test, instead of noticing that this loosely written test can be tightened to serve the purpose. So if we start with a tight test that expects the exact number of LFs at the end, we would be better off in that case, too.