unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
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







  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

  List information: https://www.gnu.org/software/emacs/

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