From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Olivier Certner Newsgroups: gmane.emacs.bugs Subject: bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable Date: Sat, 06 May 2023 23:41:29 +0200 Message-ID: <2122918.kWKFG3mEot@ravel> References: <1769719.uSAL7GYomB@ravel> <2123423.7n0gGkaxiF@ravel> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7Bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="10570"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 63286@debbugs.gnu.org To: Alan Mackenzie Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat May 06 23:42:13 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pvPfR-0002bP-I3 for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 06 May 2023 23:42:13 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pvPfH-0005ZX-VS; Sat, 06 May 2023 17:42:03 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1pvPfG-0005ZH-Bj for bug-gnu-emacs@gnu.org; Sat, 06 May 2023 17:42:02 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1pvPfG-0003Bq-2x for bug-gnu-emacs@gnu.org; Sat, 06 May 2023 17:42:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pvPfF-0000SJ-Ub for bug-gnu-emacs@gnu.org; Sat, 06 May 2023 17:42:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Olivier Certner Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 06 May 2023 21:42:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 63286 X-GNU-PR-Package: emacs Original-Received: via spool by 63286-submit@debbugs.gnu.org id=B63286.16834092961716 (code B ref 63286); Sat, 06 May 2023 21:42:01 +0000 Original-Received: (at 63286) by debbugs.gnu.org; 6 May 2023 21:41:36 +0000 Original-Received: from localhost ([127.0.0.1]:35871 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pvPep-0000Ra-H1 for submit@debbugs.gnu.org; Sat, 06 May 2023 17:41:36 -0400 Original-Received: from smtp2-g21.free.fr ([212.27.42.2]:5798) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pvPem-0000RR-Rq for 63286@debbugs.gnu.org; Sat, 06 May 2023 17:41:34 -0400 Original-Received: from ravel.localnet (unknown [90.118.140.172]) (Authenticated sender: ocert.dev@free.fr) by smtp2-g21.free.fr (Postfix) with ESMTPSA id 1EF632003BE; Sat, 6 May 2023 23:41:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=free.fr; s=smtp-20201208; t=1683409291; bh=QRCH/jJFz+dZRLXWOIhzMmF68OpkE1xq3M/0t+QHOtA=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=bnoBGOaMX4Le18rrGoNpsJgl4wnE6VuRS7RqZZMdJDC9HQpEfCSltan0uso+Y73v4 PNWZPQQe5ZrY/86i5d4BSNvnyVioiW9Gw9M+DYk0N5ni24VPsH8DYVCzgmpAi7c+Dq uilMgdDSDndOJuDWdWtOXbqVULciDBZZcP4nFVbWP0O6DIxG1+frVco3vUKkQeyXaO TUEX7o1Km0igst2FYnDqvHQg8blkeLj6taaACMmABPPCKTmnx9TidGrVUn8YhhdKzu onFsYTgIndvnBlMgxd2YskpcsgX/yNqyV+PVH/Rcgw1QWRhFmBbty98lvzf/i1HmdR ETL6tlyJ1iBQg== In-Reply-To: X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:261219 Archived-At: Hello Alan, Thanks for looking at this. > But, first things first, please. What is the problem that your change > is intended to fix? What current indentation is suboptimal? Have you read my first message? Let me copy the relevant part: "This is a proposed change adding a new style variable to control whether `for' clauses should be indented as statements or as an argument list.". I think this directly answers your first question, but let's be explicit to avoid misunderstandings: I'm not trying to "fix" anything, this is a new feature. As for the second question, I think it is safe to say it's irrelevant and you'll soon see why. I'm sorry (to you and Eli) if I didn't give more context initially, it is in part an overlook but also in part a conscious decision as a consequence of me thinking that you (both, but especially you Alan) would mostly figure out the smaller context on your own. Apparently, I was wrong. Still, I don't think I understand what is not obvious to you (Alan) in the submitted simple patch. So I'll try by giving a lot more context and details. If I don't succeed to make things crystal clear this way, I'll have to ask you to be more specific about what is still not. So, now that I'm at it, let me first explain the larger context (the one that you couldn't guess). I've been working on a CC mode style respecting guidelines ("style") and existing practice for FreeBSD's kernel. After more work I've found that this is applicable to all BSDs, with small exceptions, including 4.4BSD. The style is called Kernel Normal Form (KNF). In all BSDs (well, not the case for NetBSD, but let's ignore that here), what is inside parenthesis must, for the excess parts that are split into their own lines (in other words, for all lines other than the first, the one that contains the opening '('), be always indented by 4 columns. [Small digression: The desire to have a style for FreeBSD is why I opened Bug#63072 in the first place, but you don't need to read it (for now). In retrospect, I opened it too soon, since I noticed some discrepancies between the results of my first attempts and the indentation of some files, so this bug is currently stalled in terms of message activity. However, I've been working on it and I've almost reached my goals, so expect more CC mode tickets in the near future, for most of which Bug#63072 will be a kind of umbrella and the ultimate point. Also, the preliminary discussion with Eli in that bug is now most probably obsolete. This bug (63286) is a step in this work. While I'm here, could you also please have a look at Bug#63285, it describes a genuine problem, is self-contained (doesn't need more context) and I've provided a fix?] As for the smaller context and this particular report, let's consider a very simple `for' statement as a representative example: for (i = 0; i < n; ++i) { ... } where the block statements are elided because irrelevant to the discussion. Here are different splitting of the initial `for' line, and the desired second line's indentation in each case: for (i = 0; i < n; ++i) { for ( i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { In plain English, the second line is just always aligned 4 columns away from the start of the `for' keyword. Given the rule I stated above, all these results should be unsurprising to you. By contrast, here is the same list but showing the current indentation with the "gnu" style modified with `c-basic-offset' set to 4 to eliminate trivial style differences: for (i = 0; i < n; ++i) { for ( i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { for (i = 0; i < n; ++i) { Even more than the actual results, what is important here is the syntactic context CC Mode devises for each second line. CC Mode considers the three `for' clauses (the `i = 0', `i < n' and `++i'; technically, in the C standard, only the first is called a clause, the two others are just normal expressions, but again let's ignore for the sake of simplicity) as statements, if they stand on a line of their own. The second and third clauses are anchored to the first (indirectly for the third; it is in fact anchored to the second which itself is to the first) so are aligned with it (given that `statement' is associated to 0 in `c-offsets-alist'). As a consequence, please look at how `i < n' or `++i' line up with `i = 0'. More precisely, these statements are anchored to the first non-whitespace character of the first clause. The first clause itself, if starting on a new line, is anchored to the column following the opening `(' (in reality, it is anchored to the whole `for', but then `c-lineup-arglist-intro-after-paren' in `arglist-intro' simulates that by returning an absolute offset corresponding to the column after '('). By contrast, if a clause is split, its second part is indented as a `statement- cont' (as one would expect, and coherently with all statements that are split). As an example, please look at how the line starting with 0 is shifted by 4 columns with respect to `i'. The two main changes that are required to handle BSD KNF style for `for' statements, with "gnu" as a starting point, are: 1. `for' clauses should not be aligned to the opening parenthesis, but rather to the start of the whole `for' statement (the `f' character of the `for' keyword). 2. Clause continuations should be lined up exactly as start of clauses. Both these changes cannot be achieved by tweaking of `c-offsets-alist' with simple offsets. Indeed, for 1., there is no fixed relation between both anchors (because of the possibility of any extra whitespaces before and after the opening '('), and for 2., the clause continuation offset should be 0, meaning that currently `statement-cont' must be 0, which affects also any statement continuations not in `for' clauses, for which BSD KNF mandates 4 columns. > Have you tried fixing it by less radical means and not managed? I guess you mean "radical" because this is an engine modification? Personally, I think the proposed solution is the very opposite of radical. It doesn't disrupt any existing code at all (it's a single line change, having effect only when a new, separate style variable is set to some non-default value) and is most likely the most elegant solution. Let me briefly argue why. Given what I've written above, there are only two ways to have CC Mode follow BSD KNF style: Either I use lineups functions (or special indent hooks), or I modify the engine. I don't contest that writing a lineup function is possible. But I find it bad style in this case and for lots of reasons: 1. This is correction after the fact for what is already a special case in the engine code. What could be simpler than just avoiding it and getting the (logical and expected) fallback, that is, uniform treatment of parenthesized parts? 2. While this existing special case has its merits, it is peculiar and I don't remember seeing any other editor indenting `for' clauses like this. I've not searched bug reports, but I would be surprised that no one has already reported this behavior as a bug (which, again, personally I don't find it is; but it should be tunable). Or perhaps people just gave up on it. However, in the case of BSD KNF, this is not an option because style and the existing code are very clear about the expected outcome and such indentation won't be accepted. 3. This special case is not well integrated with other parts of `c-guess- basic-syntax'. As just a single example, in the second splitting above, the second line is analyzed as `arglist-intro'. It should arguably be recognized as `statement' instead, with some appropriate anchor. It is amusing to note that the current behavior is more coherent with the special case disabled in some places. 4. The lineup function must apply to several syntactic elements, since there are many different situations that can arise depending on how `for' clauses are split. Just for the simple example above, the listed 6 splittings for the simple `for' line lead to 4 different syntactic contexts for the second line. Most require more or less ad-hoc code to backtrack and detect that we are indeed inside a `for' clause and that the engine's chosen offset must be overridden. Take a more complex example, with parenthesized expressions in the clauses, and you have even more cases to worry about (part of them could be avoided by another lineup function that is separately useful, which I'll submit next week, but still). In fact, I would probably bet that some ~20-line lineup function could do the trick. I just think it doesn't make sense to write one: It will take more time than the proposed fix to get correctly and will be less efficient. So what are you, as the maintainer, afraid of in my proposed solution exactly? Thanks and regards. -- Olivier Certner