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: AS4713 221.184.0.0/13 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by dcvr.yhbt.net (Postfix) with ESMTP id DC6BF1F461 for ; Wed, 21 Aug 2019 15:56:02 +0000 (UTC) Received: from neon.ruby-lang.org (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id E16151209CD; Thu, 22 Aug 2019 00:55:54 +0900 (JST) Received: from o1678916x28.outbound-mail.sendgrid.net (o1678916x28.outbound-mail.sendgrid.net [167.89.16.28]) by neon.ruby-lang.org (Postfix) with ESMTPS id AC1C21209CD for ; Thu, 22 Aug 2019 00:55:51 +0900 (JST) Received: by filter0178p3mdw1.sendgrid.net with SMTP id filter0178p3mdw1-10935-5D5D698A-18 2019-08-21 15:55:54.288763826 +0000 UTC m=+339622.662735482 Received: from herokuapp.com (unknown [54.226.103.57]) by ismtpd0040p1iad2.sendgrid.net (SG) with ESMTP id jfQtngDiQHKuRh88M-C1sg for ; Wed, 21 Aug 2019 15:55:54.147 +0000 (UTC) Date: Wed, 21 Aug 2019 15:55:54 +0000 (UTC) From: merch-redmine@jeremyevans.net Message-ID: References: Mime-Version: 1.0 X-Redmine-MailingListIntegration-Message-Ids: 70011 X-Redmine-Project: ruby-trunk X-Redmine-Issue-Id: 16112 X-Redmine-Issue-Author: k0kubun X-Redmine-Sender: jeremyevans0 X-Mailer: Redmine X-Redmine-Host: bugs.ruby-lang.org X-Redmine-Site: Ruby Issue Tracking System X-Auto-Response-Suppress: All Auto-Submitted: auto-generated X-SG-EID: =?us-ascii?Q?RVE3t853K5scBhbmJHUzZTFFeVC=2FZSUmHZ0Dc+26wcEi2CTgsF1oz0wTSSxGGN?= =?us-ascii?Q?BIMGXN6y6YYBaxTyD76yPFPfjmu7kIkjekkP=2FbX?= =?us-ascii?Q?z95+QNQm01hGPq8DLWchpVl6wPUDOEtfArTaB8P?= =?us-ascii?Q?uDzsiKBBVYq3SAT+mblJhtIt6mb=2Fnxohg6EZ2HF?= =?us-ascii?Q?BML7OuMUXx5DB1emR5mzvGCH7o4spwcLhqg=3D=3D?= To: ruby-core@ruby-lang.org X-ML-Name: ruby-core X-Mail-Count: 94462 Subject: [ruby-core:94462] [Ruby master Misc#16112] Reduce the possibility of "expand tabs" commit occurrences X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" Issue #16112 has been updated by jeremyevans0 (Jeremy Evans). k0kubun (Takashi Kokubun) wrote: > Thanks for your comment, Jeremy. Your opinion makes sense except: > > > 2) I'm against this. If we do keep the "expand tabs", then it should be applied to modified lines as well as new lines, so that we move in the direction of fewer tabs. > > Can you tell me what problem you want to solve by not taking this option, compared to the current situation (not other options)? I do think both the current situation and option 2 solve the problem that "we move in the direction of fewer tabs", so I just failed to interpret your reasoning. Basically, I am in favor of removing the "expand tabs" commits (stop trying to enforce whitespace). However, if we do keep the "expand tabs" commits (keep trying to enforce whitespace), I think we should continue to apply the the enforcement to both modified and new lines. The only reason to enforce whitespace is to eventually get to a point where whitespace is mostly consistent, and if we do not enforce it for modified lines, I don't think we will get there. > In my understanding, option 2 is a weaker version or a subset of option 0. Therefore, if you agree with option 0 but disagree with option 2, you or I may not be understanding how the other thinks. I can see what you are saying. And in a linear system, it makes sense. However, in my viewpoint, this is a non-linear system, and it is possible for a middle point (option 2) to be suboptimal to either end point (option 0 and keeping things as they are). > Whenever we make a commit which adds new lines, which is the case for many of our commits, option 2 will increase the ratio of space-indented lines in the repository. In that sense it helps "we move in the direction of fewer tabs" for sure. Some commits are not reducing the number of tab-indented lines, but is that different from the current "expand tabs"? It is true that the ratio will be increased. However, in absolute numbers, it does not necessarily move in the direction of fewer tabs (unless tab lines are deleted). All this being said, I'm not strongly against option 2. I'm more against 4, 5, and 1. > By combining option 3 with the current "expand tabs" behavior (changing indentation of changed lines to spaces as well, probably you felt it's not the case with option 2? If so, it's not my intention) and option 2 with the weaker "expand tabs" behavior, I believe we can move in the direction of fewer tabs for sure. I like option 3 because it is optional and something I would use. I don't personally like using tabs. ---------------------------------------- Misc #16112: Reduce the possibility of "expand tabs" commit occurrences https://bugs.ruby-lang.org/issues/16112#change-80896 * Author: k0kubun (Takashi Kokubun) * Status: Open * Priority: Normal * Assignee: ---------------------------------------- ## Problem * While we agreed to use only spaces for indentation of C code in [Bug #14246], we sometimes hit "expand tabs" commit when we just edit a part of existing lines with hard tab indentation. * "expand tabs" commit bothers people when we need to perform a revert or a backport. * However, because [Bug #14246] aimed to eventually make indentations consistent, we do not want to just drop "expand tabs". * One of the motivations to solve [Bug #14246] is that having hard tabs makes preprocessed MJIT header ugly and it makes debugging on GCC hard. As MJIT may introduce C-code inlining for sources outside vm.c in the future, we want to fix the issue in almost all C sources which can be run on runtime. ## Possible Solutions There would be some options to approach the problem. I'd like to hear opinions about these options. 1. Expand all tabs at once for all files managed by auto-style. * In [Bug #14246], this was clearly objected for the reason "Indents should become consistent over time". * In my understanding, not folliwng "Indents should become consistent over time" would be problematic mainly for polluting "git blame" and conflicts on backport. * For the first point, we can use `-w` option of `git blame` to ignore that. * For backport, it's a trade-off with many "expand tabs" commits. We need opinions about this from @usa and @nagachika . * Also reverting a commit before the commit expanding all tabs would be bothering, for a short while. 2. Skip expand tabs for existing lines when indentation is not changed, and expand tabs only in newly-added lines. * If editors are configured properly, "expand tabs" would not happen for new patches in this approach. * Even in this approach, at least we will not go to the opposite direction of eventually achieving [Bug #14246]. So it seems acceptable. 3. Prepare a local pre-commit hook to expand tabs in newly-added or edited lines, and let people who do not want an "expand tabs" commit use it. 4. Make pull request CI fail when there's diff to be expanded, and let people who do not want "expand tabs" commit things from pull requests. ---Files-------------------------------- Screen Shot 2019-08-19 at 2.43.06 PM.png (48.8 KB) -- https://bugs.ruby-lang.org/