From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A243FC433E0 for ; Thu, 14 May 2020 09:47:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6F50B206A5 for ; Thu, 14 May 2020 09:47:52 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="giwU6QKW" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725970AbgENJrv (ORCPT ); Thu, 14 May 2020 05:47:51 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47940 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1725935AbgENJrv (ORCPT ); Thu, 14 May 2020 05:47:51 -0400 Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C20DEC061A0C for ; Thu, 14 May 2020 02:47:50 -0700 (PDT) Received: by mail-wr1-x441.google.com with SMTP id i15so2997492wrx.10 for ; Thu, 14 May 2020 02:47:50 -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=DZaUVTJ5z6MzpuY7hKu5zJRp6t9j+DchVMEBY7C4qsc=; b=giwU6QKWBJiYsAC1uODgGKnEW5mRRbTbqSybdbeNwVWSBIEXMwmYI/LnmKf9ieU63d 6h0pWqyo+fo3fKsUefbwpqwXvbFGqVMsh4wwoodAzsYElR6l2P3e+TAp8DwZKKKjlpFL e7wT5k6tfwKDKVNUxnb9fZsJBMXoXD8nXKHY9ZMaa0f84o1pM/sq/AQqId2Gc5VIO7es sKS0yueq6IuQr/9VY/4NxCNKmBWeAIyJBbym4OHJHcAeVD1/kKR7UP0IG/F0Xd/lTpJZ KUi6gVKPH2IcmxP4lSBVqXC5B3ETi8hmhycV5hJcX8OGVL6T9aH9cFab2m/4U1zEUlTv IAlQ== 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=DZaUVTJ5z6MzpuY7hKu5zJRp6t9j+DchVMEBY7C4qsc=; b=nqlBb9lBbZnGtKajP+aazhDhfeqoqRtU07Bhvz7n5fMx3pdvdlsjL5CrSxHitIXeuH MXHFRTJF/H0xp3mSKa9Tuf8mdXwOEK69HUpS81bOUwZI+Fcg7MElbDYxDAyvCjTeMu5s G8hABQllde3S5I7RlhCH8OmjgXayO0+2wokLpySzTMJNmpfK9QbrCLr/+TvGkq91CQ4Q tiqE9fpSz5JEPxuh8xQo1lzbo3TNGVJcX3MI34+YIr//e+IN2axIq3gyR4PU9LUZdjZp oOBGsTDDfnyYo5MOtyb9vDRYygyuAeFbmaRV35HOVILyEqKxl+4VsVYds5osJ9iVzhCQ xiIg== X-Gm-Message-State: AOAM533lNDceaNrtunp+1Cw1BelTAW20QEjxa0OsUq788noFDDQb5Yjc TXx6lM2DUxXPRWjX8Q8ZZX7rzgwo X-Google-Smtp-Source: ABdhPJyeJe5XIeOb6WmBiYTRNHMrZ8RppwvQzXrT33XafEYZETzEhm85Rqu2Cg5wIshmUE93Qbmf3A== X-Received: by 2002:a5d:400f:: with SMTP id n15mr4671715wrp.419.1589449668997; Thu, 14 May 2020 02:47:48 -0700 (PDT) Received: from [192.168.1.201] (155.20.198.146.dyn.plus.net. [146.198.20.155]) by smtp.googlemail.com with ESMTPSA id 89sm3352743wrj.37.2020.05.14.02.47.47 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 May 2020 02:47:48 -0700 (PDT) Subject: Re: [PATCH v2 1/5] rebase -i: add --ignore-whitespace flag To: Elijah Newren , Phillip Wood Cc: Johannes Schindelin , Junio C Hamano , Rohit Ashiwal , Git Mailing List References: <20200407141125.30872-1-phillip.wood123@gmail.com> <20200429102521.47995-1-phillip.wood123@gmail.com> <20200429102521.47995-2-phillip.wood123@gmail.com> From: Phillip Wood Message-ID: Date: Thu, 14 May 2020 10:47:47 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.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 13/05/2020 04:54, Elijah Newren wrote: > Sorry for taking so long to get back to you, and thanks for pushing > this forward. Thanks for taking a look at this series > > On Wed, Apr 29, 2020 at 3:26 AM Phillip Wood wrote: >> >> From: Rohit Ashiwal >> >> Rebase is implemented with two different backends - 'apply' and 'merge' >> each of which support a different set of options. In particuar the apply >> backend supports a number of options implemented by 'git am' that are >> not available to the merge backend. As part of an on going effort to >> remove the apply backend this patch adds support for the >> --ignore-whitespace option to the merge backend. This option treats >> lines with only whitespace changes as unchanged and is implemented in >> the merge backend by translating it to -Xignore-space-change. >> >> Signed-off-by: Rohit Ashiwal >> Signed-off-by: Phillip Wood >> [...] >> --- >> Documentation/git-rebase.txt | 12 +++- >> builtin/rebase.c | 19 ++++-- >> t/t3422-rebase-incompatible-options.sh | 1 - >> t/t3436-rebase-more-options.sh | 86 ++++++++++++++++++++++++++ >> 4 files changed, 111 insertions(+), 7 deletions(-) >> create mode 100755 t/t3436-rebase-more-options.sh >> >> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt >> index f7a6033607..d060c143e6 100644 >> --- a/Documentation/git-rebase.txt >> +++ b/Documentation/git-rebase.txt >> @@ -422,8 +422,16 @@ your branch contains commits which were dropped, this option can be used >> with `--keep-base` in order to drop those commits from your branch. >> >> --ignore-whitespace:: >> + Behaves differently depending on which backend is selected. > > I still don't like this wording; it defers answering the question, > implies that the difference is intentional, and most importantly > provides no context about *intended* behavior. I tried to communicate > this to Rohit multiple times, but he seemed to fixate on and highlight > the differences in a way that made them sound like they were by > design, rather than highlighting the intent we want to move towards > and mentioning that this patch gets us most the way there. > > As far as I can tell, the --ignore-whitespace and > -Xignore-space-change were always meant to do the same thing: ignore > differences in whitespace when doing so can avoid conflicts. > > In case anyone isn't sure about my assertion that these were always > meant to do the same thing: > * apply aliases --ignore-whitespace and --ignore-space-change; they > meant the same thing > * commit f008cef4ab ("Merge branch 'jc/apply-ignore-whitespace'", > 2014-06-03) says that apply's --ignore-space-change wasn't behaving > consistently with diff's --ignore-space-change > * diff's --ignore-space-change goes through xdiff's XDL opts, much > like merge-recursive does. > Further, the original commit that introduced these xdiff options to > merge-recursive, 4e5dd044c6 ("merge-recursive: options to ignore > whitespace changes", 2010-08-26), it is clear that: > * he only cared about ignore-space-at-eol and implemented > ignore-space-change at the same time only for completeness > * it wouldn't matter to his usecase if whitespace-only changes were > stripped, thus he wouldn't have spotted the bug it has > * the wording also suggests these options were picked to match > options of the same name elsewhere in git > > I would rather we said something like: > Ignore whitespace differences when trying to reconcile > differences. Currently, each backend implements an approximation of > this behavior: > >> ++ >> +apply backend: When applying a patch, ignore changes in whitespace in >> +context lines. > > Maybe add something like: > (Unfortunately, this means that if the "old" lines being replaced > by the patch differ only in whitespace from the existing file, you > will get a merge conflict instead of a successful patch application.) > >> ++ >> +merge backend: Treat lines with only whitespace changes as unchanged >> +when merging. > > Maybe add something like: > (Unfortunately, this means that any patch hunks that were intended > to modify whitespace and nothing else will be dropped, even if the > other side had no changes that conflicted.) Thanks for the suggestions, I'll incorporate them into the re-roll. Best Wishes Phillip >> + >> --whitespace=