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=AWL,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 36F741FA04 for ; Wed, 13 May 2020 03:55:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727082AbgEMDy5 (ORCPT ); Tue, 12 May 2020 23:54:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50540 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1725898AbgEMDy5 (ORCPT ); Tue, 12 May 2020 23:54:57 -0400 Received: from mail-oo1-xc41.google.com (mail-oo1-xc41.google.com [IPv6:2607:f8b0:4864:20::c41]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D8822C061A0C for ; Tue, 12 May 2020 20:54:55 -0700 (PDT) Received: by mail-oo1-xc41.google.com with SMTP id t3so3182259oou.8 for ; Tue, 12 May 2020 20:54:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=IhnOD/5UpjgdUn3WFC2fmwm0Bxe+sJModsTswde1/Yc=; b=VBVmhQbrFh5VDCU9YlVZlWPUvBNHbMRHGpFi2VdRjrK+b5bqmbahbPUllaTTbDf0+i TYRPNXGndoA0Ek9wwI3w7Vxdtq4C+QLL5SYFnuPLvRe2lHXLsg8s//ncUC8vgHjD0p+b L19uxU+hyZBzOadAOCA/WSff02Ds0SSbMPrNrxvzVWXiVxfzXs5glfpYEwbqfRYU406t MmHmByO0rcnF1KXFDLCFQAHOANY6gEaRw5N01zSl7U8LoPXTpJcNmjEsvL8tnnK7fUIv IXG1dBOjw9nN+h6+Uv0aFgHfPZiu4mvZOwQ31UOQD05IDxEVWjg57AJYiB2fez06eQsY nFtg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=IhnOD/5UpjgdUn3WFC2fmwm0Bxe+sJModsTswde1/Yc=; b=Tjyk6lmT5QH0UGQIYKDYKYg141hItqhjtkpoO1sL2hvvy/wudEslcWCHQaCZELYmb6 Yxr7BvjIl/L8mC733td14IFWPWx2qw+8s77SNk81AwcRYs4Bfbq2iuTQIEMm/cuz3T1l O0Kz7Bz7U09B1w1BdDGfbNl180jgrm84OQGkQjzNDC7v+fWV8X2GQ02TXPFPNJ13xqDY 7HAI2r74rh0hJPImLdBjxfWcTOuOL7J+vbRWA66jfaKtnEGeYjSJjA1H8auZQHqjDwnu j9A6W5wg3EXxkXIyGCQZRcym3pbE1NSg60lwhwOm4O35N5MFXr+qwnEhNGNT6YLfuVx8 R3XQ== X-Gm-Message-State: AGi0PuZP0HzywT3WpgmKuBPKIixyl4u38hGZlsXXhazPI6FCj9xVGzVQ JoocbZUdClFa14ogUwwGYaACrSII2TM3Mw/8hZ0= X-Google-Smtp-Source: APiQypJ+ZdSJfymg2h0X3ugscx8WoWxZPiBIPlY9SPK/s8Bfs8V3G9Q3dCOgUHckVJ/Cgtw7PgBYlY6guZpoal3ee6w= X-Received: by 2002:a4a:a11d:: with SMTP id i29mr12205740ool.7.1589342094446; Tue, 12 May 2020 20:54:54 -0700 (PDT) MIME-Version: 1.0 References: <20200407141125.30872-1-phillip.wood123@gmail.com> <20200429102521.47995-1-phillip.wood123@gmail.com> <20200429102521.47995-2-phillip.wood123@gmail.com> In-Reply-To: <20200429102521.47995-2-phillip.wood123@gmail.com> From: Elijah Newren Date: Tue, 12 May 2020 20:54:43 -0700 Message-ID: Subject: Re: [PATCH v2 1/5] rebase -i: add --ignore-whitespace flag To: Phillip Wood Cc: Johannes Schindelin , Junio C Hamano , Rohit Ashiwal , Git Mailing List Content-Type: text/plain; charset="UTF-8" Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Sorry for taking so long to get back to you, and thanks for pushing this forward. 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.) > + > --whitespace=