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: Tue, 09 May 2023 12:08:55 +0200 Message-ID: <1769415.c5zRbCDrAu@ravel> References: <1769719.uSAL7GYomB@ravel> <2122918.kWKFG3mEot@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="26381"; 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 Tue May 09 12:10:14 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 1pwKIQ-0006i3-15 for geb-bug-gnu-emacs@m.gmane-mx.org; Tue, 09 May 2023 12:10:14 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1pwKIG-0007WP-8k; Tue, 09 May 2023 06:10:04 -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 1pwKIE-0007W5-JN for bug-gnu-emacs@gnu.org; Tue, 09 May 2023 06:10: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 1pwKIE-0000du-BC for bug-gnu-emacs@gnu.org; Tue, 09 May 2023 06:10:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1pwKIE-0006Sz-78 for bug-gnu-emacs@gnu.org; Tue, 09 May 2023 06:10:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Olivier Certner Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 09 May 2023 10:10:02 +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.168362694224777 (code B ref 63286); Tue, 09 May 2023 10:10:02 +0000 Original-Received: (at 63286) by debbugs.gnu.org; 9 May 2023 10:09:02 +0000 Original-Received: from localhost ([127.0.0.1]:42474 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pwKHF-0006RH-IR for submit@debbugs.gnu.org; Tue, 09 May 2023 06:09:02 -0400 Original-Received: from smtp2-g21.free.fr ([212.27.42.2]:43582) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1pwKHC-0006R8-UU for 63286@debbugs.gnu.org; Tue, 09 May 2023 06:09:00 -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 4AFA02005A9; Tue, 9 May 2023 12:08:56 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=free.fr; s=smtp-20201208; t=1683626937; bh=KTGVNO29VkY58tb6byv9Uy5RyUNdsf8/dwH/uq4/zLY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=sNAP3FFul3QpLjpC4+yjcfFRyHvDcyU+as7wGuk1DjTmStvmA7eBP7tmj9jooyzvf ozCMHvPVLP7MxsJzL/s5sN8jReaF3rlD+R2JAFwYNYkQQ3d6iT/fLQknciTtTp4b2M 6XFsIw0K1RwnoxKXcQDFXp5TS20htpt6P9Ih0FzMD8UzPPpPHpoKw2WhteN/wPIbz/ nfry1jnL6/REjCxAR6OqE8llwgAEiaRB/xj8qMQ7GzGFIce6SMa7EKxyo85xpFUcDI meJdZQ8vC6rsk+XxzxYiZlCf7FVEbmz+xYfQ8PDwL9E9MFKWRV8mvfTjH4iv1m50Ss ygzNAnHgC7ejA== 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:261399 Archived-At: Hello Alan, > > I guess you mean "radical" because this is an engine modification? > > Not really. It proposes introducing a new style variable, for one > thing. But it also proposes making the syntactic expressions produced > for some constructs non-constant, and that where there isn't a bug to > solve. It proposes a new style variable, in addition to the already existing 16 (!). Syntactic contexts are constant as long as the variable keeps the same value, but yes, they differ depending on the particular chosen value, and that's exactly the point of this new setting. This is no different than how `c- syntactic-indentation-in-macros' works. Finally, design and architecture don't necessary need to be held up until there are actual bugs to solve. > I don't find it elegant. Elegance in this matter, at least in my book, is reaching objectives with the minimal amounts of code at the right place(s) while preserving or enhancing the design (that's perhaps a reductive definition, but it is enough for the current discussion). I wouldn't have bothered submitting this patch to the engine if it didn't enhance the current architecture. And it does so because it allows a logical change in behavior with a single switch at the right place rather than tweaking many small pieces after the fact to achieve it. > The style variables are intended for widely used features in CC Mode. I'm generally skeptical on most popularity contexts since they are usually biased, either statistically or conceptually. But I'll contribute an additional data point: I certainly did not have to use any of them in 20+ years until recently, except for `c-basic-offset' and `c-offsets-alist' of course. If you're stating a policy, then I don't think it is written anywhere in the doc or in code comments. And I think the current situation may just be proof of its non-existence. If it's a new one, than I hope I can convince you it shouldn't apply in this case. > To introduce another one just for a particular indentation would disrupt > the logical consistency of CC Mode. Would "disrupt the logical consistency of CC Mode"? Nothing less? Could you elaborate on what is the consistency that this change threatens exactly? I can only hope that you won't be exactly repeating the points I already responded to above, since they are unconvincing at best and currently appear to be great exaggerations. On the other hand more explanations would be more than welcome. > There have been many fixes made to indentation over the years, and if > even just a few of these involved adding new style variables, the style > system would be more confusing and less usable that it is. There are changes and changes. I would not be surprised that most don't call for a new style variable. This is simply not the case here. > It is also easy, particularly in the current case. CC Mode is designed > to make it easy. You could do this with a single lineup function which > would return either the desired offset, or nil when the function is > inapplicable. The c-offsets-alist entries for the three (or four?) > pertinent syntactic symbols would then be modified to lists beginning > with the symbol `first' and followed by the new function then the > default. The new function would likely be well under 20 lines (not > counting comments). See the page "c-offsets-alist" in the CC Mode > manual. I know the manual in and out, and the code realizing the indentation very well. I've also written a bunch of non-trivial lineup functions (comparable to some bundled in `cc-align.el') and I'm already using the excellent combining facilities provided by offset lists. And with that knowledge, I have to disagree. Writing lineup functions is not easy, it can be tedious and messy, even if in the end the code doesn't need to be very long. Because of a certain lack of coherency in how syntactic contexts are built (and lack of sufficient documentation for that process), it is indeed easy to forget cases, which only appear later when indenting some new or yet unexplored code. Sure, you can write lineups very quickly for toy examples, but this is not what I'm aiming at (and you're now aware of the larger context). I don't claim to have years of experience with the internals of CC mode and lineups, contrary to you. Yet, in addition to what I already exposed, I've carefully analyzed a large part of CC Mode code in the past days (I've not counted, but based on the number of files and their size, very roughly between 30 and 50% of the code base), that's also why I'm confidently making these statements. I might be wrong. On the other hand, I'm also beginning to wonder if your ~20-year maintainership of CC Mode is simply biasing you on the matter. Indeed, don't you see any problem with: "The c-offsets-alist entries for the three (or four?) pertinent syntactic symbols would then be modified to lists beginning with the symbol `first' and followed by the new function then the default." It doesn't have to be like that. And it is not going to take only a single `first' list. And we're only talking about a single issue in the journey to implement BSD KNF, there are a lot more requiring even more complexities in offsets and new lineup functions. I know that since I have working code for the whole thing. > The treatment is already uniform - it just doesn't currently allow the > indentation you want in your new style. Here we are definitely leaving the realm of points of view or opinions for that of plain facts. And unless we do not have the same words, this statement is just completely wrong. ;; CASE 7D: we are inside a conditional test clause. treat ;; these things as statements ((progn (goto-char containing-sexp) (and (c-safe (c-forward-sexp -1) t) (looking-at "\\[^_]"))) (goto-char (1+ containing-sexp)) (c-forward-syntactic-ws indent-point) (if (eq char-before-ip ?\;) (c-add-syntax 'statement (point)) (c-add-syntax 'statement-cont (point)) )) See the `looking-at' call matching the `for' keyword? If this is not special casing, I don't know what this is. Case 7D is a sub-case of case 7, whose aim is to match expressions, not statements. Following subcases (7F to 7G) actually parse parenthesized expressions (I'm omitting 7E which concerns ObjC only). So, this case specifically matches a `for' on the path to parsing parenthesized expressions, and in a branch which is not supposed to parse statements. QED. > No, there have been no other bug reports about this in the last 20 years > or so, at least. Yours is the first one. Then at least there won't be things repeated. But this gives me even more responsibility, speaking up for all those that didn't voice their concern. Which, now that I vaguely recall, probably included me over 20 years ago in a very different context. And, again, I know of no C editor behaving the same as Emacs+CC Mode on `for' clauses, and certainly not the popular ones (but I also do not know all of them, and have not used some for years; please someone point out to those that would do). > The treatment of `for' in CC Mode is not a special case in CC Mode. Sorry, but it simply is. See above (the part commenting case 7D) and just below (next paragraph). > If anything, it's a special case in the language definitions, where > executable code is enclosed in parentheses. This is also plain wrong. There's no more "executable code" (this sentence is not used anywhere in the standard) in `for' clauses than in expressions (parenthesized or not). In fact, all but the first clause are just expressions in the standard, no more. Only the first can be a declaration instead. None of the clauses are statements. You're mixing things up. I encourage you to read the standard so you don't have to take my word for it. > I think there are rather _several_ different situations rather than many. You're playing on words but I think you've got the idea. The main point is that this complexity is unnecessary. Still, you're underestimating the number of corner cases. > I don't foresee the troubles that you do. By the time the lineup > function comes to be run, things like extra parenthesised expressions > have already been analysed by the indentation engine. The anchor > positions allow one readily to find the opening parenthesis and the > preceding `for' token, or determine that they don't exist in the current > case. Not only I foresee them, but I've already experienced some of them. And I already know what you're saying. It would be perfect if that was all there was to it. Unfortunately, it's not the case. You're omitting for example the "special case" of having nested parenthesized groups with opening parentheses all on the same line. > I think a single 20 line function could do the trick, as outlined above. > The extra run time (if any) taken would be too small to measure. May be, I'm not sure. CC Mode is already very slow for full file indentation (I know it's not its goal, but still). But whether that matters or not, what is certain is that implementing a special case in an engine to subsequently undo it in a hook is simply bloat and a waste of cycles, however small it is. > That it would fragment CC Mode, reducing its cohesiveness and > introducing special cases where none are needed. It would entail > changes to the test suite, which would complicate it, and substantial > changes to the documentation. I couldn't disagree more. What you are proposing is a lineup function that is itself essentially a large `cond' of special cases just to undo an engine's very localized special case. The proposed approach instead just selectively deactivates the latter in the first place. Adding only a few new tests is more than enough, because there is provably no change at all if the variable is not explicitly set and because changes are confined to `for' clauses only. How does this "complicate" the suite? If adding a few tests is a problem, then the test suite is the problem. Addendum: I cannot find anything in the source tree but a mere 'cc-mode- tests.el' that is 111 lines long (including comments). Where is the "suite" you're talking about? Even if you want to be comprehensive, on this precise change there's not much more to document than what is already in the patch. Of course, a lot should be added to explain all the special cases of the engine, but this is independent from this report. Again, you're talking about "substantial changes in the documentation", but you don't mention what you have in mind. Finally, as for "fragmentation" or threat to "cohesiveness", I'm sorry to have to say that I have yet to read a single precise and relevant argument that can really back this point of view. > In short, I don't think the approach you've presented is the right one > to get the indentation that you need. Then I'm asking you to step back and then reconsider, because you're most likely wrong. At the very least, filling the gaps between the vague high- level principles you stated (and which I generally support) and how the proposed change actually breaks them, as well as answering my other questions, would be very welcome. If you're not willing to, then I'm not sure there is any point in continuing this discussion. Regards. -- Olivier Certner