From: Olivier Certner <ocert.dev@free.fr>
To: Alan Mackenzie <acm@muc.de>
Cc: 63286@debbugs.gnu.org
Subject: bug#63286: 30.0.50; CC Mode: New `c-for-clauses-as-arglist' style variable
Date: Wed, 10 May 2023 15:20:40 +0200 [thread overview]
Message-ID: <7485977.bDI8rlcXoi@ravel> (raw)
In-Reply-To: <ZFpihG90DTc1oUaW@ACM>
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
next prev parent reply other threads:[~2023-05-10 13:20 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
2023-05-10 13:20 ` Olivier Certner [this message]
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=7485977.bDI8rlcXoi@ravel \
--to=ocert.dev@free.fr \
--cc=63286@debbugs.gnu.org \
--cc=acm@muc.de \
/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.