unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
@ 2023-05-04 22:19 Olivier Certner
  2023-05-04 22:22 ` Olivier Certner
  0 siblings, 1 reply; 12+ messages in thread
From: Olivier Certner @ 2023-05-04 22:19 UTC (permalink / raw)
  To: 63286

Hi,

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.

Patch to be attached once the bug is created.

Regards.

-- 
Olivier Certner







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-04 22:19 bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable Olivier Certner
@ 2023-05-04 22:22 ` Olivier Certner
  2023-05-05  4:57   ` Eli Zaretskii
  2023-05-05 11:14   ` Alan Mackenzie
  0 siblings, 2 replies; 12+ messages in thread
From: Olivier Certner @ 2023-05-04 22:22 UTC (permalink / raw)
  To: 63286

[-- Attachment #1: Type: text/plain, Size: 37 bytes --]

Proposed change.

-- 
Olivier Certner

[-- Attachment #2: 0001-CC-Mode-New-c-for-clauses-as-arglist-style-variable.patch --]
[-- Type: text/x-patch, Size: 6248 bytes --]

From 3a84abb4cb06f9673c851aa44f2f1b27752560ef Mon Sep 17 00:00:00 2001
From: Olivier Certner <olce.emacs@certner.fr>
Date: Wed, 3 May 2023 18:12:27 +0200
Subject: [PATCH] CC Mode: New `c-for-clauses-as-arglist' style variable

This new style variable allows to disable special handling of "for"
statements' clauses, which is that there are indented as statements,
or continuations of such, when on separate lines instead of argument
lists or continuations of such.

* lisp/progmodes/cc-engine.el (c-guess-basic-syntax): Skip case 7D,
which specifically handles the above-mentioned case.  While here,
rewrite the case's comment to be more explicit about what it does.

* lisp/progmodes/cc-vars.el (c-for-clauses-as-arglist): The new style
variable.
(c-style-variables): Include the new style variable.
(c-style-variables-are-local-p): Update documentation following
addition of the variable.

* doc/misc/cc-mode.texi (Style Variables): List the new variable.
(Syntactic Symbols): Indicate precisely which syntactic symbol can
appear in a syntactic element when analyzing `for' clauses depending
on the style variable value.

(Bug#63286)
---
 doc/misc/cc-mode.texi       | 19 ++++++++++++++-----
 lisp/progmodes/cc-engine.el | 13 +++++++------
 lisp/progmodes/cc-vars.el   | 14 +++++++++++++-
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/doc/misc/cc-mode.texi b/doc/misc/cc-mode.texi
index 71bf3fcee4a..0e4ba4c6a91 100644
--- a/doc/misc/cc-mode.texi
+++ b/doc/misc/cc-mode.texi
@@ -2624,6 +2624,7 @@ Style Variables
 Commas});@*
 @code{c-cleanup-list} (@pxref{Clean-ups});@*
 @code{c-basic-offset} (@pxref{Customizing Indentation});@*
+@code{c-for-clauses-as-arglist} (@pxref{Syntactic Symbols});@*
 @code{c-offsets-alist} (@pxref{c-offsets-alist});@*
 @code{c-comment-only-line-offset} (@pxref{Comment Line-Up});@*
 @code{c-special-indent-hook}, @code{c-label-minimum-indentation}
@@ -4267,7 +4268,8 @@ Syntactic Symbols
 Subsequent lines in an enum or static array list where the line begins
 with an open brace.  @ref{Brace List Symbols}.
 @item statement
-A statement.  @ref{Function Symbols}.
+A statement, including `for' clauses except if
+@code{c-for-clauses-as-arglist} is true.  @ref{Function Symbols}.
 @item statement-cont
 A continuation of a statement.  @ref{Function Symbols}.
 @item annotation-var-cont
@@ -4309,15 +4311,22 @@ Syntactic Symbols
 @item comment-intro
 A line containing only a comment introduction.  @ref{Literal Symbols}.
 @item arglist-intro
-The first line in an argument list.  @ref{Paren List Symbols}.
+The first line in an argument list or a parenthesized expression.
+Note that @code{for} clauses are rather considered statements (or
+their continuation) except if @code{c-for-clauses-as-arglist} is true.
+@ref{Paren List Symbols}.
 @item arglist-cont
 Subsequent argument list lines when no arguments follow on the same
-line as the arglist opening paren.  @ref{Paren List Symbols}.
+line as the arglist opening paren.  Same remark concerning @code{for}
+clauses as for @code{arglist-intro} above.  @ref{Paren List Symbols}.
 @item arglist-cont-nonempty
 Subsequent argument list lines when at least one argument follows on
-the same line as the arglist opening paren.  @ref{Paren List Symbols}.
+the same line as the arglist opening paren.  Same remark concerning
+@code{for} clauses as for @code{arglist-intro} above.  @ref{Paren List
+Symbols}.
 @item arglist-close
-The solo close paren of an argument list.  @ref{Paren List Symbols}.
+The solo close paren of an argument list or a @code{for} clause.
+@ref{Paren List Symbols}.
 @item stream-op
 Lines continuing a stream operator (C++ only).  @ref{Literal
 Symbols}. @c @emph{FIXME!!!  Can this not be moved somewhere better?}
diff --git a/lisp/progmodes/cc-engine.el b/lisp/progmodes/cc-engine.el
index 27740b4903c..409cbc59ab5 100644
--- a/lisp/progmodes/cc-engine.el
+++ b/lisp/progmodes/cc-engine.el
@@ -15246,12 +15246,13 @@ c-guess-basic-syntax
 			     (c-most-enclosing-brace paren-state (point))
 			     paren-state))
 
-	 ;; 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 "\\<for\\>[^_]")))
+	 ;; CASE 7D: We are inside a for clause.  Treat these clauses
+	 ;; as statements unless `c-for-clauses-as-arglist' is
+	 ;; non-nil.
+	 ((and (not c-for-clauses-as-arglist)
+	       (goto-char containing-sexp)
+	       (c-safe (c-forward-sexp -1) t)
+	       (looking-at "\\<for\\>[^_]"))
 	  (goto-char (1+ containing-sexp))
 	  (c-forward-syntactic-ws indent-point)
 	  (if (eq char-before-ip ?\;)
diff --git a/lisp/progmodes/cc-vars.el b/lisp/progmodes/cc-vars.el
index 72d4b93ee59..304ebca52fe 100644
--- a/lisp/progmodes/cc-vars.el
+++ b/lisp/progmodes/cc-vars.el
@@ -158,7 +158,8 @@ c-style-variables
     c-comment-prefix-regexp c-doc-comment-style c-cleanup-list
     c-hanging-braces-alist c-hanging-colons-alist
     c-hanging-semi&comma-criteria c-backslash-column c-backslash-max-column
-    c-special-indent-hook c-label-minimum-indentation c-offsets-alist)
+    c-special-indent-hook c-label-minimum-indentation
+    c-for-clauses-as-arglist c-offsets-alist)
   "List of the style variables.")
 
 (defvar c-fallback-style nil)
@@ -960,6 +961,16 @@ c-label-minimum-indentation
   :type 'integer
   :group 'c)
 
+(defcustom-c-stylevar c-for-clauses-as-arglist nil
+  "Whether to consider for clauses as part of an argument list.
+The clauses of the for statement are normally considered by CC
+mode as separate statements when at start of a line \(and
+statement continuations when split).  Setting this variable to
+non-nil indicates that they should be treated as any other
+argument lists."
+  :type 'boolean
+  :group 'c)
+
 (defcustom c-progress-interval 5
   "Interval used to update progress status during long re-indentation.
 If a number, percentage complete gets updated after each interval of
@@ -1449,6 +1460,7 @@ c-style-variables-are-local-p
     `c-backslash-column'
     `c-backslash-max-column'
     `c-label-minimum-indentation'
+    `c-for-clauses-as-arglist'
     `c-offsets-alist'
     `c-special-indent-hook'
     `c-indentation-style'"
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-04 22:22 ` Olivier Certner
@ 2023-05-05  4:57   ` Eli Zaretskii
  2023-05-06 21:49     ` Olivier Certner
  2023-05-05 11:14   ` Alan Mackenzie
  1 sibling, 1 reply; 12+ messages in thread
From: Eli Zaretskii @ 2023-05-05  4:57 UTC (permalink / raw)
  To: Olivier Certner, Alan Mackenzie; +Cc: 63286

> From: Olivier Certner <ocert.dev@free.fr>
> Date: Fri, 05 May 2023 00:22:12 +0200
> 
> This new style variable allows to disable special handling of "for"
> statements' clauses, which is that there are indented as statements,
> or continuations of such, when on separate lines instead of argument
> lists or continuations of such.

I don't think I understand the feature you are proposing.  Can you
elaborate, perhaps with examples?

> --- a/doc/misc/cc-mode.texi
> +++ b/doc/misc/cc-mode.texi
> @@ -2624,6 +2624,7 @@ Style Variables
>  Commas});@*
>  @code{c-cleanup-list} (@pxref{Clean-ups});@*
>  @code{c-basic-offset} (@pxref{Customizing Indentation});@*
> +@code{c-for-clauses-as-arglist} (@pxref{Syntactic Symbols});@*
>  @code{c-offsets-alist} (@pxref{c-offsets-alist});@*
>  @code{c-comment-only-line-offset} (@pxref{Comment Line-Up});@*
>  @code{c-special-indent-hook}, @code{c-label-minimum-indentation}
> @@ -4267,7 +4268,8 @@ Syntactic Symbols
>  Subsequent lines in an enum or static array list where the line begins
>  with an open brace.  @ref{Brace List Symbols}.
>  @item statement
> -A statement.  @ref{Function Symbols}.
> +A statement, including `for' clauses except if
> +@code{c-for-clauses-as-arglist} is true.  @ref{Function Symbols}.
>  @item statement-cont
>  A continuation of a statement.  @ref{Function Symbols}.
>  @item annotation-var-cont
> @@ -4309,15 +4311,22 @@ Syntactic Symbols
>  @item comment-intro
>  A line containing only a comment introduction.  @ref{Literal Symbols}.
>  @item arglist-intro
> -The first line in an argument list.  @ref{Paren List Symbols}.
> +The first line in an argument list or a parenthesized expression.
> +Note that @code{for} clauses are rather considered statements (or
> +their continuation) except if @code{c-for-clauses-as-arglist} is true.
> +@ref{Paren List Symbols}.
>  @item arglist-cont
>  Subsequent argument list lines when no arguments follow on the same
> -line as the arglist opening paren.  @ref{Paren List Symbols}.
> +line as the arglist opening paren.  Same remark concerning @code{for}
> +clauses as for @code{arglist-intro} above.  @ref{Paren List Symbols}.
>  @item arglist-cont-nonempty
>  Subsequent argument list lines when at least one argument follows on
> -the same line as the arglist opening paren.  @ref{Paren List Symbols}.
> +the same line as the arglist opening paren.  Same remark concerning
> +@code{for} clauses as for @code{arglist-intro} above.  @ref{Paren List
> +Symbols}.
>  @item arglist-close
> -The solo close paren of an argument list.  @ref{Paren List Symbols}.
> +The solo close paren of an argument list or a @code{for} clause.
> +@ref{Paren List Symbols}.
>  @item stream-op
>  Lines continuing a stream operator (C++ only).  @ref{Literal
>  Symbols}. @c @emph{FIXME!!!  Can this not be moved somewhere better?}

This uses @ref incorrectly.  The original text also uses it
incorrectly.  Alan, please fix that when you have time: those should
be @xref, not @ref.  The latter is not appropriate at the beginning of
a sentence.

> +(defcustom-c-stylevar c-for-clauses-as-arglist nil
> +  "Whether to consider for clauses as part of an argument list.

"for" should be quoted here, to make this sentence more clear.

> +The clauses of the for statement are normally considered by CC
> +mode as separate statements when at start of a line \(and
> +statement continuations when split).  Setting this variable to
> +non-nil indicates that they should be treated as any other
> +argument lists."

I don't understand what "argument lists" have to do with 'for'.  Or
maybe I don't understand what you mean by "for clauses".

> +  :type 'boolean
> +  :group 'c)

Defcustoms should have the :version tag.

Finally, would it be possible to have tests for this feature?

Thanks.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-04 22:22 ` Olivier Certner
  2023-05-05  4:57   ` Eli Zaretskii
@ 2023-05-05 11:14   ` Alan Mackenzie
  2023-05-06 21:41     ` Olivier Certner
  1 sibling, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2023-05-05 11:14 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 63286, acm

Hello, Olivier.

Thanks for working on this.

On Fri, May 05, 2023 at 00:22:12 +0200, Olivier Certner wrote:
> Proposed change.

But, first things first, please.  What is the problem that your change
is intended to fix?  What current indentation is suboptimal?  Have you
tried fixing it by less radical means and not managed?

> -- 
> Olivier Certner

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-05 11:14   ` Alan Mackenzie
@ 2023-05-06 21:41     ` Olivier Certner
  2023-05-08 15:02       ` Alan Mackenzie
  0 siblings, 1 reply; 12+ messages in thread
From: Olivier Certner @ 2023-05-06 21:41 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 63286

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







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-05  4:57   ` Eli Zaretskii
@ 2023-05-06 21:49     ` Olivier Certner
  0 siblings, 0 replies; 12+ messages in thread
From: Olivier Certner @ 2023-05-06 21:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 63286, Alan Mackenzie

Hello Eli,

> I don't think I understand the feature you are proposing.  Can you
> elaborate, perhaps with examples?

Sure.  Please see my reply to Alan.

> This uses @ref incorrectly.  The original text also uses it
> incorrectly.  Alan, please fix that when you have time: those should
> be @xref, not @ref.  The latter is not appropriate at the beginning of
> a sentence.
> 
> "for" should be quoted here, to make this sentence more clear.
>
> Defcustoms should have the :version tag.

Indeed.  I'm taking note.  But all these are minutiae that are less pressing 
than more high-level issues that might change the submission significantly, so 
I'll wait for the appropriate moment to actually do the changes if there are 
still applicable then.

> I don't understand what "argument lists" have to do with 'for'.  Or
> maybe I don't understand what you mean by "for clauses".

Then you should definitely read my answer to Alan.
 
> Finally, would it be possible to have tests for this feature?

Of course.  But again, this is not the most pressing issue right now.

Regards.

-- 
Olivier Certner







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-06 21:41     ` Olivier Certner
@ 2023-05-08 15:02       ` Alan Mackenzie
  2023-05-09 10:08         ` Olivier Certner
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2023-05-08 15:02 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 63286

Hello, Olivier.

On Sat, May 06, 2023 at 23:41:29 +0200, Olivier Certner wrote:
> 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.

In your message with the patch, there was a lot of "what", but very
little, if any, "why".  Thanks for filling in the gap for me!

[ .... ]

> 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.

[ .... ]

> 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) {

[ .... ]

> 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:

[ .... ]

> 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?  

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.

> 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.

I don't find it elegant.  The style variables are intended for widely
used features in CC Mode.  To introduce another one just for a
particular indentation would disrupt the logical consistency of CC Mode.
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.

> 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 lineup functions (or special indent hooks), or I 
> modify the engine.  I don't contest that writing a lineup function is 
> possible.

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.

> 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?

The treatment is already uniform - it just doesn't currently allow the
indentation you want in your new style.

> 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.

No, there have been no other bug reports about this in the last 20 years
or so, at least.  Yours is the first one.

> 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.

The treatment of `for' in CC Mode is not a special case in CC Mode.  If
anything, it's a special case in the language definitions, where
executable code is enclosed in parentheses.

> 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.

I think there are rather _several_ different situations rather than many.

> 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).

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.

> 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.

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.

> So what are you, as the maintainer, afraid of in my proposed solution
> exactly?

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.

In short, I don't think the approach you've presented is the right one
to get the indentation that you need.

> Thanks and regards.


> -- 
> Olivier Certner

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-08 15:02       ` Alan Mackenzie
@ 2023-05-09 10:08         ` Olivier Certner
  2023-05-09 15:11           ` Alan Mackenzie
  0 siblings, 1 reply; 12+ messages in thread
From: Olivier Certner @ 2023-05-09 10:08 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 63286

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 "\\<for\\>[^_]")))
	  (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







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-09 10:08         ` Olivier Certner
@ 2023-05-09 15:11           ` Alan Mackenzie
  2023-05-10 13:20             ` Olivier Certner
  0 siblings, 1 reply; 12+ messages in thread
From: Alan Mackenzie @ 2023-05-09 15:11 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 63286, acm

Hello, Olivier.

On Tue, May 09, 2023 at 12:08:55 +0200, Olivier Certner wrote:

[ .... ]

> 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).

The part of the design that your patch would disrupt would be the
principle of the two stage indentation.  The first stage determines the
syntactic context, and the second stage indents it according to the
current style.  Your patch would put style dependent things into the
first stage.

[ .... ]

> > 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.

You will have used style variables implicitly in constructing your own
styles.  They are central to the entire style mechanism.

And yes, there are lots of conventions in CC Mode (and indeed in Emacs
as a whole) which are implicit.  There is no maintainers' manual.

[ .... ]

> > 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.

The current problem isn't materially different from all the other
indentation problems in years past.

[ .... ]

> 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.

That can indeed be the case, but it isn't the case here.  Just to press
home this point, I threw the following (untested) together in a few
minutes.  It needs a doc string and probably some comments, but could
easily serve as the basis for your new lineup function:

(defun c-lineup-KNF-for (langelem)
  (save-excursion
    (goto-char (c-langelem-pos langelem))
    (when
        (or (looking-at "\\<for\\>\\([^_]\\|$\\)")
            (and
             (or (eq (char-after) ?\()
                 (zerop (c-backward-token-2)))
             (eq (char-after) ?\()
             (zerop (c-backward-token-2))
             (looking-at "\\<for\\>\\([^_]\\|$\\)")))
      (vector (+ (current-column) c-basic-offset)))))

..  Forgive me pointing out that your proposed patch, by contrast, is
over 100 lines 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).

Yes, when coding, you've got to take care.  This is true even for the
"simplest" of tasks.

> 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, one is either ignorant/inexperienced, or is biased by a long
period spent doing something.  That is an inescapable conundrum.

[ .... ]

> > 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.

If such other programmers actually exist.

> 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).
 
Emacs has been at the forefront of editor development for nearly 50
years.  In many respects, other editors have caught up, but not in all
respects.  ;-)

[ .... ]

> > 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 think I am underestimating them, having tended to lineup
functions many times before.

> > 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.

Which brings me back to a question I asked earlier, and I don't think
you answered:  Have you already tried to implement the indentation you
want using, e.g., lineup functions?  If so, where did that try hit
problems?

[ .... ]

> 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.

There would be substantial changes to the large nested cond form which
constitutes c-guess-basic-syntax.  The pertinent topic of what happens
to those occurences which would no longer trigger CASE 7D hasn't even
come up for discussion, yet.

> 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?

In the upstream project at SourceForge, see
https://cc-mode.sourceforge.net/hgaccess.php.

> 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.

I have in mind the doc changes in your proposed patch.  I haven't even
started considering to what extent they would be needed, or need
expanding upon.

> 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.

Well, I've tried to explain this over several posts here.  I'm sorry
I've not succeeded.  Maybe the subject of restricting the two parts of
indentation to their current separate functionalities (see above) might
have done the trick.

> > 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.

For the reasons I've tried to explain, I won't be using your proposed
patch.  That's just the way things are.  Sorry.

> Regards.

> -- 
> Olivier Certner

-- 
Alan Mackenzie (Nuremberg, Germany).





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-09 15:11           ` Alan Mackenzie
@ 2023-05-10 13:20             ` Olivier Certner
  2023-05-10 14:26               ` Eli Zaretskii
  2023-09-11 18:14               ` Stefan Kangas
  0 siblings, 2 replies; 12+ messages in thread
From: Olivier Certner @ 2023-05-10 13:20 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: 63286

Hello Alan,

> The part of the design that your patch would disrupt would be the
> principle of the two stage indentation.  The first stage determines the
> syntactic context, and the second stage indents it according to the
> current style.  Your patch would put style dependent things into the
> first stage.

You didn't really need to state that, the two stages are fairly obvious, 
although this separation is counterbalanced in part by the fact that the sole 
purpose of syntactic analysis is in the end to apply indentation.

I think that you didn't get what I meant.  I'll try one last time, with a 
different presentation.  My patch doesn't put style-dependent things into the 
first stage.  It makes CC Mode optionally consider `for' clauses as arglist 
instead of statements (to say it shortly).  This isn't per-se style-related at 
all, and I've already pointed out that this behavior matches better what the 
language standard describes, whether in spirit or form (a part you didn't 
respond to; have you looked up the standard? having a CC Mode maintainer that 
doesn't seem to know this kind of basic stuff about C is worrying).  It is 
true that initially I was trying to solve an indentation problem, but that 
doesn't make it the reason behind the proposed change, it was merely a 
trigger.  As I foresaw myself from the start, it is possible to solve it with 
lineup functions, but again, that's not the point.  That the proposed change 
allows to easily achieve the goal is an additional sign that it is the most 
appropriate.  Maybe I wasn't clear enough.  In any case, the end result is 
that you're confusing goal, mean and reason.

> You will have used style variables implicitly in constructing your own
> styles.  They are central to the entire style mechanism.

I've used them no more than users that don't need the functionality would 
"use" `c-for-clauses-as-arglist' by just ignoring its existence.
 
> That can indeed be the case, but it isn't the case here.  Just to press
> home this point, I threw the following (untested) together in a few
> minutes.  It needs a doc string and probably some comments, but could
> easily serve as the basis for your new lineup function:
> 
> (defun c-lineup-KNF-for (langelem)
>   (save-excursion
>     (goto-char (c-langelem-pos langelem))
>     (when
>         (or (looking-at "\\<for\\>\\([^_]\\|$\\)")
>             (and
>              (or (eq (char-after) ?\()
>                  (zerop (c-backward-token-2)))
>              (eq (char-after) ?\()
>              (zerop (c-backward-token-2))
>              (looking-at "\\<for\\>\\([^_]\\|$\\)")))
>       (vector (+ (current-column) c-basic-offset)))))

Thanks.  This could have been helpful to me, but it comes too late since I 
have one already (along the same lines).  Of course, this doesn't work with 
parenthesized expressions inside the clauses (which the patch submitted in 
this bug doesn't handle either, in all fairness).  But I've already been 
having a solution for that for days.

> ..  Forgive me pointing out that your proposed patch, by contrast, is
> over 100 lines long.

Alan, what you're only succeeding with this is in making a fool out of 
yourself.  I hope you'll realize it.

100 lines long?  Gosh...  Have you ever heard about "context lines" in diffs?

In your lineup function, you omitted the docstring, comments and 
documentation.  Also, while it doesn't need real integration with the rest of 
CC Mode, being a lineup function, it does need being mentioned in the style 
(for 5 different syntactic symbols), adding even more lines.  How 
convenient...

By the same standard, my patch can be reduced to only *two* lines, a single 
added `and' clause to case 7D (forget about the `progn' simplification) and 
the `defvar' for the style variable.

Thus, forgive me for pointing out that your patch is at least x6 larger than 
mine (and I'm giving you a large extra for not counting style integration).  
Although not particularly complex, your function still has 2 calls to check 
for regular expressions and a bunch of conditionals that are probably opaque 
to any newcomer (which maybe you've realized I'm not?), the former because of 
the `arglist-close' discrepancy I mentioned in a former message.  No wonder CC 
Mode is so slow on full file indentation.  If you can't see possible 
improvements there, than I guess that "everything is for the best in the best 
of all possible worlds".

> Yes, when coding, you've got to take care.  This is true even for the
> "simplest" of tasks.

You don't teach your grandmother to suck eggs.  And you're missing the point 
(again).

> Indeed, one is either ignorant/inexperienced, or is biased by a long
> period spent doing something.  That is an inescapable conundrum.

You may have a binary mind.  But please don't make a generality out of your 
case.
 
> I don't think I am underestimating them, having tended to lineup
> functions many times before.

Given the above, I think you've unfortunately not drawn high-level lessons 
from such experiences.

> Which brings me back to a question I asked earlier, and I don't think
> you answered:  Have you already tried to implement the indentation you
> want using, e.g., lineup functions?  If so, where did that try hit
> problems?

"[...] I know that since I have working code for the whole thing."

I've already reported incoherencies or obstacles in my previous messages.  You 
must have missed them.  I overcome them a while back on my own.  I initially 
didn't report a precise count of line for a single lineup since I had in fact 
several of them playing together to reach the final goal, including achieving 
the desired indentation in this precise case.  I did this exercise in 
isolation yesterday, coming up with something similar to yours, just a bit 
simpler (hint: regular expressions are unnecessary).  Anyway, you can't beat 
the actual two lines of code that are the meat of the patch.  But this was not 
my main point either.

> There would be substantial changes to the large nested cond form which
> constitutes c-guess-basic-syntax.  The pertinent topic of what happens
> to those occurences which would no longer trigger CASE 7D hasn't even
> come up for discussion, yet.

No, there wouldn't at all.  Mind you, I looked at what happens when case 7D is 
not triggered before proposing the patch, and I'm afraid there is absolutely 
nothing to change there.  So it is most likely FUD, again.  If you have some 
concrete objections on the matter, feel free to present them.

> In the upstream project at SourceForge, see
> https://cc-mode.sourceforge.net/hgaccess.php.

Thanks.  I don't think this will be terribly useful now for me, but I'm taking 
note.
 
> I have in mind the doc changes in your proposed patch.  I haven't even
> started considering to what extent they would be needed, or need
> expanding upon.

Then, let me point out that my doc changes are ~7 lines added.  Have you read 
them?  I you find that to be "substantial changes in the documentation"...
 
> Maybe the subject of restricting the two parts of indentation to their
> current separate functionalities (see above) might have done the trick.

Well, that's certainly better than what you initially offered, but that's 
still far from cutting it, I'm sorry (see my first two paragraphs, at top).
 
> That's just the way things are.

No, that's just the way you are making them.  And it's fine for me, I have my 
own Emacs build with my own customizations, including for CC Mode (and at some 
point, as time permits, probably my own mode for C).

You've repeatedly shown that you don't have a clue about what this report was 
really about.  That's no so fine for CC Mode, but you're the one in charge as 
the maintainer.  At least I now know it is useless to make such reports to 
you.  So I'll limit myself to down-to-earth ones, such as Bug#63285, which 
hopefully should well be in your reach.

You can close this bug.

-- 
Olivier Certner







^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-10 13:20             ` Olivier Certner
@ 2023-05-10 14:26               ` Eli Zaretskii
  2023-09-11 18:14               ` Stefan Kangas
  1 sibling, 0 replies; 12+ messages in thread
From: Eli Zaretskii @ 2023-05-10 14:26 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 63286, acm

> Cc: 63286@debbugs.gnu.org
> From: Olivier Certner <ocert.dev@free.fr>
> Date: Wed, 10 May 2023 15:20:40 +0200
> 
> Alan, what you're only succeeding with this is in making a fool out of 
> yourself.  I hope you'll realize it.
> [...]
> 100 lines long?  Gosh...  Have you ever heard about "context lines" in diffs?
> [...]
> You don't teach your grandmother to suck eggs.  And you're missing the point 
> (again).
> [...]
> You may have a binary mind.  But please don't make a generality out of your 
> case.
>  [...]
> Given the above, I think you've unfortunately not drawn high-level lessons 
> from such experiences.
> [...]
> You've repeatedly shown that you don't have a clue about what this report was 
> really about.

Please in the future try to make your points without ad-hominem
attacks on your opponents.  Thanks in advance.





^ permalink raw reply	[flat|nested] 12+ messages in thread

* bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
  2023-05-10 13:20             ` Olivier Certner
  2023-05-10 14:26               ` Eli Zaretskii
@ 2023-09-11 18:14               ` Stefan Kangas
  1 sibling, 0 replies; 12+ messages in thread
From: Stefan Kangas @ 2023-09-11 18:14 UTC (permalink / raw)
  To: Olivier Certner; +Cc: 63286-done, Alan Mackenzie

Olivier Certner <ocert.dev@free.fr> writes:

> You've repeatedly shown that you don't have a clue about what this report was
> really about.  That's no so fine for CC Mode, but you're the one in charge as
> the maintainer.  At least I now know it is useless to make such reports to
> you.  So I'll limit myself to down-to-earth ones, such as Bug#63285, which
> hopefully should well be in your reach.
>
> You can close this bug.

Thanks, done.





^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-09-11 18:14 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-04 22:19 bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable Olivier Certner
2023-05-04 22:22 ` Olivier Certner
2023-05-05  4:57   ` Eli Zaretskii
2023-05-06 21:49     ` Olivier Certner
2023-05-05 11:14   ` Alan Mackenzie
2023-05-06 21:41     ` Olivier Certner
2023-05-08 15:02       ` Alan Mackenzie
2023-05-09 10:08         ` Olivier Certner
2023-05-09 15:11           ` Alan Mackenzie
2023-05-10 13:20             ` Olivier Certner
2023-05-10 14:26               ` Eli Zaretskii
2023-09-11 18:14               ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).