all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Alan Mackenzie <acm@muc.de>
To: Olivier Certner <ocert.dev@free.fr>
Cc: 63286@debbugs.gnu.org, acm@muc.de
Subject: bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
Date: Tue, 9 May 2023 15:11:00 +0000	[thread overview]
Message-ID: <ZFpihG90DTc1oUaW@ACM> (raw)
In-Reply-To: <1769415.c5zRbCDrAu@ravel>

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





  reply	other threads:[~2023-05-09 15:11 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2023-05-10 13:20             ` Olivier Certner
2023-05-10 14:26               ` Eli Zaretskii
2023-09-11 18:14               ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZFpihG90DTc1oUaW@ACM \
    --to=acm@muc.de \
    --cc=63286@debbugs.gnu.org \
    --cc=ocert.dev@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.