bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

[Liran Zvibel]

Hi,

I’m not subscribed to this list (or to -devel), so please reply also to my email when responding.

Thanks,
Liran Zvibel.

** Description:

Fix extra indent of d-mode  "else static if" statements 

The D programming language has a notion of “static if” conditionals.
The d-mode (from https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode.git)
requires cc-mode.
When writing “else static if” blocks, the code block is getting indented twice,
as well as all future “else static if”s that come later. This is very annoying.

This simple fix was originally suggested here: 
http://www.prowiki.org/wiki4d/wiki.cgi?EditorSupport/EmacsDMode/ElseStaticIf
The simple fix treats "static if" same as “if" that comes right after an “else".

I fixed it locally in my installed emacs long time ago, but today when downloading 
trunk to test 24.4 I was disappointed it was not already fixed by someone else. 
I know many D programmers that apply this change locally to their installed Emacs, 
hopefully not for long.

** ChangeLog

2014-07-31 Liran Zvibel <liranz@gmail.com>

	* Small cc-mode change to make sure “else static if” does not get 
	deeper and deeper indentation the same way that “else if” is treated 
	for d-mode that requires cc-mode.

** The patch :

=== modified file 'lisp/progmodes/cc-engine.el'
*** lisp/progmodes/cc-engine.el	2014-06-29 11:26:47 +0000
--- lisp/progmodes/cc-engine.el	2014-07-31 15:22:15 +0000
*************** comment at the start of cc-engine.el for
*** 9053,9061 ****
  			     (looking-at "else\\>[^_]")
  			     (save-excursion
  			       (goto-char old-pos)
! 			       (looking-at "if\\>[^_]")))
  			;; Special case to avoid deeper and deeper indentation
! 			;; of "else if" clauses.
  			)

  		       ((and (not stop-at-boi-only)
--- 9053,9062 ----
  			     (looking-at "else\\>[^_]")
  			     (save-excursion
  			       (goto-char old-pos)
!                                (or (looking-at "if\\>[^_]")
! 				   (looking-at "static\\>[^_]"))))
  			;; Special case to avoid deeper and deeper indentation
! 			;; of "else if"/"static else if" clauses.
  			)

  		       ((and (not stop-at-boi-only)

bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

[Stefan Kangas]

Hi Alan,

Could you please help review also the below patch for cc-engine.el?

Thanks in advance.

Best regards,
Stefan Kangas

Liran Zvibel <liranz@gmail.com> writes:

> Hi,
>
> I’m not subscribed to this list (or to -devel), so please reply also to my email when responding.
>
> Thanks,
> Liran Zvibel.
>
> ** Description:
>
> Fix extra indent of d-mode  "else static if" statements 
>
> The D programming language has a notion of “static if” conditionals.
> The d-mode (from https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode.git)
> requires cc-mode.
> When writing “else static if” blocks, the code block is getting indented twice,
> as well as all future “else static if”s that come later. This is very annoying.
>
> This simple fix was originally suggested here: 
> http://www.prowiki.org/wiki4d/wiki.cgi?EditorSupport/EmacsDMode/ElseStaticIf
> The simple fix treats "static if" same as “if" that comes right after an “else".
>
> I fixed it locally in my installed emacs long time ago, but today when downloading 
> trunk to test 24.4 I was disappointed it was not already fixed by someone else. 
> I know many D programmers that apply this change locally to their installed Emacs, 
> hopefully not for long.
>
> ** ChangeLog
>
> 2014-07-31 Liran Zvibel <liranz@gmail.com>
>
> 	* Small cc-mode change to make sure “else static if” does not get 
> 	deeper and deeper indentation the same way that “else if” is treated 
> 	for d-mode that requires cc-mode.
>
> ** The patch :
>
> === modified file 'lisp/progmodes/cc-engine.el'
> *** lisp/progmodes/cc-engine.el	2014-06-29 11:26:47 +0000
> --- lisp/progmodes/cc-engine.el	2014-07-31 15:22:15 +0000
> *************** comment at the start of cc-engine.el for
> *** 9053,9061 ****
>   			     (looking-at "else\\>[^_]")
>   			     (save-excursion
>   			       (goto-char old-pos)
> ! 			       (looking-at "if\\>[^_]")))
>   			;; Special case to avoid deeper and deeper indentation
> ! 			;; of "else if" clauses.
>   			)
>
>   		       ((and (not stop-at-boi-only)
> --- 9053,9062 ----
>   			     (looking-at "else\\>[^_]")
>   			     (save-excursion
>   			       (goto-char old-pos)
> !                                (or (looking-at "if\\>[^_]")
> ! 				   (looking-at "static\\>[^_]"))))
>   			;; Special case to avoid deeper and deeper indentation
> ! 			;; of "else if"/"static else if" clauses.
>   			)
>
>   		       ((and (not stop-at-boi-only)

bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

[Alan Mackenzie]

Hello, Stefan and Liran.

On Mon, Jan 20, 2020 at 22:18:05 +0100, Stefan Kangas wrote:
> Hi Alan,

> Could you please help review also the below patch for cc-engine.el?

I'm less than happy about putting a special purpose workaround into a
critical bit of CC Mode (c-add-stmt-syntax) without having even seen the
problem.

Liran, if you're still there and still interested, could you possibly
supply me with a sample of D source code containing the problem?  I
would hope to be able to enhance CC Mode to handle it in a more general
and useful fashion.

> Thanks in advance.

> Best regards,
> Stefan Kangas

-- 
Alan Mackenzie (Nuremberg, Germany).



> Liran Zvibel <liranz@gmail.com> writes:

> > Hi,
> >
> > I’m not subscribed to this list (or to -devel), so please reply also to my email when responding.
> >
> > Thanks,
> > Liran Zvibel.
> >
> > ** Description:
> >
> > Fix extra indent of d-mode  "else static if" statements 
> >
> > The D programming language has a notion of “static if” conditionals.
> > The d-mode (from https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode.git)
> > requires cc-mode.
> > When writing “else static if” blocks, the code block is getting indented twice,
> > as well as all future “else static if”s that come later. This is very annoying.
> >
> > This simple fix was originally suggested here: 
> > http://www.prowiki.org/wiki4d/wiki.cgi?EditorSupport/EmacsDMode/ElseStaticIf
> > The simple fix treats "static if" same as “if" that comes right after an “else".
> >
> > I fixed it locally in my installed emacs long time ago, but today when downloading 
> > trunk to test 24.4 I was disappointed it was not already fixed by someone else. 
> > I know many D programmers that apply this change locally to their installed Emacs, 
> > hopefully not for long.
> >
> > ** ChangeLog
> >
> > 2014-07-31 Liran Zvibel <liranz@gmail.com>
> >
> > 	* Small cc-mode change to make sure “else static if” does not get 
> > 	deeper and deeper indentation the same way that “else if” is treated 
> > 	for d-mode that requires cc-mode.
> >
> > ** The patch :
> >
> > === modified file 'lisp/progmodes/cc-engine.el'
> > *** lisp/progmodes/cc-engine.el	2014-06-29 11:26:47 +0000
> > --- lisp/progmodes/cc-engine.el	2014-07-31 15:22:15 +0000
> > *************** comment at the start of cc-engine.el for
> > *** 9053,9061 ****
> >   			     (looking-at "else\\>[^_]")
> >   			     (save-excursion
> >   			       (goto-char old-pos)
> > ! 			       (looking-at "if\\>[^_]")))
> >   			;; Special case to avoid deeper and deeper indentation
> > ! 			;; of "else if" clauses.
> >   			)
> >
> >   		       ((and (not stop-at-boi-only)
> > --- 9053,9062 ----
> >   			     (looking-at "else\\>[^_]")
> >   			     (save-excursion
> >   			       (goto-char old-pos)
> > !                                (or (looking-at "if\\>[^_]")
> > ! 				   (looking-at "static\\>[^_]"))))
> >   			;; Special case to avoid deeper and deeper indentation
> > ! 			;; of "else if"/"static else if" clauses.
> >   			)
> >
> >   		       ((and (not stop-at-boi-only)

bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

[Liran Zvibel]

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

@("notrace") JSONValue objToJSON(T)(auto ref const T obj) @trusted {
    alias U = Unqual!T;
    static if (is(U == JSONValue)) {
        return obj;
    }
    else static if (__traits(compiles, obj.toJSON())) {
        return obj.toJSON();
    }
    else static if (is(U == typeof(null))) {
        return JSONValue(null);
    }
    else static if (is(U == enum)) { // before the JSONValue casts as int
        return JSONValue(obj.to!string);
    }
    else static if (isSomeString!U) {
        if (obj is null) {
            return JSONValue(null);
        } else {
            return JSONValue(obj);
        }
    }
    else static if(is(U == SysTime)) {
        if (obj == SysTime.init) {
            return JSONValue(null);
        } else {
            return JSONValue(obj.toUTC().toISOExtString());
        }
    }
    else static if(is(U == Duration)) {
        auto hns = obj.total!"hnsecs"();
        if (hns % 10_000_000 == 0) {
            return JSONValue(hns / 10_000_000);
        }
        else {
            return JSONValue(hns / 10_000_000.0);
        }
    }
    else static if (is(U == UUID)) {
        return JSONValue(obj.toString());
    }
    else static if (isFloatingPoint!U) {
        if (obj != obj || obj == U.infinity || obj == -U.infinity) {
            return JSONValue(null);
        }
        else {
            auto tmp = cast(long)obj;
            if (tmp == obj) {
                // dump as long, to avoid losing percision
                return JSONValue(tmp);
            }
            else {
                return JSONValue(obj);
            }
        }
    }
:


> On Jan 26, 2020, at 7:29 AM, Alan Mackenzie <acm@muc.de> wrote:
> 
> Hello, Stefan and Liran.
> 
> On Mon, Jan 20, 2020 at 22:18:05 +0100, Stefan Kangas wrote:
>> Hi Alan,
> 
>> Could you please help review also the below patch for cc-engine.el?
> 
> I'm less than happy about putting a special purpose workaround into a
> critical bit of CC Mode (c-add-stmt-syntax) without having even seen the
> problem.
> 
> Liran, if you're still there and still interested, could you possibly
> supply me with a sample of D source code containing the problem?  I
> would hope to be able to enhance CC Mode to handle it in a more general
> and useful fashion.
> 
>> Thanks in advance.
> 
>> Best regards,
>> Stefan Kangas
> 
> -- 
> Alan Mackenzie (Nuremberg, Germany).
> 
> 
> 
>> Liran Zvibel <liranz@gmail.com> writes:
> 
>>> Hi,
>>> 
>>> I’m not subscribed to this list (or to -devel), so please reply also to my email when responding.
>>> 
>>> Thanks,
>>> Liran Zvibel.
>>> 
>>> ** Description:
>>> 
>>> Fix extra indent of d-mode  "else static if" statements 
>>> 
>>> The D programming language has a notion of “static if” conditionals.
>>> The d-mode (from https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode.git)
>>> requires cc-mode.
>>> When writing “else static if” blocks, the code block is getting indented twice,
>>> as well as all future “else static if”s that come later. This is very annoying.
>>> 
>>> This simple fix was originally suggested here: 
>>> http://www.prowiki.org/wiki4d/wiki.cgi?EditorSupport/EmacsDMode/ElseStaticIf
>>> The simple fix treats "static if" same as “if" that comes right after an “else".
>>> 
>>> I fixed it locally in my installed emacs long time ago, but today when downloading 
>>> trunk to test 24.4 I was disappointed it was not already fixed by someone else. 
>>> I know many D programmers that apply this change locally to their installed Emacs, 
>>> hopefully not for long.
>>> 
>>> ** ChangeLog
>>> 
>>> 2014-07-31 Liran Zvibel <liranz@gmail.com>
>>> 
>>> 	* Small cc-mode change to make sure “else static if” does not get 
>>> 	deeper and deeper indentation the same way that “else if” is treated 
>>> 	for d-mode that requires cc-mode.
>>> 
>>> ** The patch :
>>> 
>>> === modified file 'lisp/progmodes/cc-engine.el'
>>> *** lisp/progmodes/cc-engine.el	2014-06-29 11:26:47 +0000
>>> --- lisp/progmodes/cc-engine.el	2014-07-31 15:22:15 +0000
>>> *************** comment at the start of cc-engine.el for
>>> *** 9053,9061 ****
>>>   			     (looking-at "else\\>[^_]")
>>>   			     (save-excursion
>>>   			       (goto-char old-pos)
>>> ! 			       (looking-at "if\\>[^_]")))
>>>   			;; Special case to avoid deeper and deeper indentation
>>> ! 			;; of "else if" clauses.
>>>   			)
>>> 
>>>   		       ((and (not stop-at-boi-only)
>>> --- 9053,9062 ----
>>>   			     (looking-at "else\\>[^_]")
>>>   			     (save-excursion
>>>   			       (goto-char old-pos)
>>> !                                (or (looking-at "if\\>[^_]")
>>> ! 				   (looking-at "static\\>[^_]"))))
>>>   			;; Special case to avoid deeper and deeper indentation
>>> ! 			;; of "else if"/"static else if" clauses.
>>>   			)
>>> 
>>>   		       ((and (not stop-at-boi-only)


[-- Attachment #2: Type: text/html, Size: 24780 bytes --]

bug#18158: Fix extra indent of d-mode "else static if" statements in cc-engine.el

[Alan Mackenzie]

Hello, Liran.

Many thanks for the sample D file.  I can indeed reproduce the problem,
and I'm looking into ways of fixing it properly, thus removing the need
for the workarounds which have found their way into d-mode.el.

On Tue, Jan 28, 2020 at 17:26:41 -0800, Liran Zvibel wrote:
> @("notrace") JSONValue objToJSON(T)(auto ref const T obj) @trusted {
>     alias U = Unqual!T;
>     static if (is(U == JSONValue)) {
>         return obj;
>     }
>     else static if (__traits(compiles, obj.toJSON())) {
>         return obj.toJSON();
>     }
>     else static if (is(U == typeof(null))) {

[ .... ]

>             }
>         }
>     }
> :


> > On Jan 26, 2020, at 7:29 AM, Alan Mackenzie <acm@muc.de> wrote:

> > Hello, Stefan and Liran.

> > On Mon, Jan 20, 2020 at 22:18:05 +0100, Stefan Kangas wrote:
> >> Hi Alan,

> >> Could you please help review also the below patch for cc-engine.el?

> > I'm less than happy about putting a special purpose workaround into a
> > critical bit of CC Mode (c-add-stmt-syntax) without having even seen the
> > problem.

> > Liran, if you're still there and still interested, could you possibly
> > supply me with a sample of D source code containing the problem?  I
> > would hope to be able to enhance CC Mode to handle it in a more general
> > and useful fashion.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#18158: D Mode: Getting rid of the ugly advice on looking-at.

[Alan Mackenzie]

Hello, Russel and Vladimir.

I am the maintainer of the CC Mode project at Sourceforge, and also CC
Mode within Emacs.  I hope you don't mind too much me writing to your
personal Email addresses - the submission of bug reports to Github is
restricted to those with a Github account, and I don't have and don't
want a Github account.

In the newest released d-mode.el (version 201812050604) at around line
698, advice is added to the function looking-at, such that (looking-at
""if\\>[^_]") will also return t on "static if" (and a couple of other
things, too).  This was done because of a lack of a needed feature in
the base CC Mode.

This advice is not ideal.  If nothing else, it will be inconvenient
should the regexp "if\\>[^_]" ever change to, say, "if\\_>".  I am
intending to make a change like this throughout CC Mode sometime, since
"\\_<" and "\\_>" have existed in Emacs's regexp engine for at least 10
years, now.

So, why not add suitable features to CC Mode so that this advice in
d-mode.el is not needed any more?  In the patches below, I have
introduced two new c-lang-defconsts, c-post-else-if-prefix-kwds (which
should take the value '("static") in D Mode) and
c-post-else-no-indent-kwds (with value '("if" "debug" "version") in D
Mode).  c-add-stmt-syntax in cc-engine.el has been modified to use
(c-lang-defvars derived from) these variables in place of the hard coded
"if\\>[^_]".

I suggest an amendment to d-mode.el along the lines of my patch below.
It is intended to work with both the new CC Mode, and existing CC Mode
versions, using a compile time test.  At some time in the future, it
should be possible to get rid of the old advice mechanism entirely.

I propose to commit the patch for CC Mode, below.  I would be grateful
indeed if either of you could find the time to review it, first.

Just a quick tip, though you probably don't need it: when compiling the
new d-mode.el, be sure that the new cc-langs.elc has been loaded into
your Emacs.

So, here is the suggested patch for d-mode.el:



--- d-mode.201812050604.el	2018-12-05 06:07:51.000000000 +0000
+++ d-mode.el	2020-02-02 11:31:41.724734790 +0000
@@ -334,6 +334,21 @@
   d '("for" "if" "switch" "while" "catch" "synchronized" "scope"
       "foreach" "foreach_reverse" "with" "unittest"))
 
+(cc-eval-when-compile
+  (when (c-safe (c-lang-const c-post-else-no-indent-kwds java))
+
+    (c-lang-defconst c-post-else-if-prefix-kwds
+    ;; Keywords which can come between "else" and "if" (or some other member
+    ;; of `c-post-else-no-indent-kwds') without giving the substatement being
+    ;; introduced extra indentation."
+      d '("static"))
+
+    (c-lang-defconst c-post-else-no-indent-kwds
+      ;; Keywords which when following an "else" (possibly with intervening
+      ;; members of `c-post-else-if-prefix-kwds') do not cause extra
+      ;; indentation of the substatement being introduced.
+      d '("if" "debug" "version"))))
+
 (c-lang-defconst c-simple-stmt-kwds
   ;; Statement keywords followed by an expression or nothing.
   d '("break" "continue" "goto" "return" "throw"))
@@ -695,32 +710,35 @@
 
 ;;----------------------------------------------------------------------------
 ;;; Workaround for special case of 'else static if' not being handled properly
-(defun d-special-case-looking-at (orig-fun &rest args)
-  ;; checkdoc-params: (orig-fun args)
-  "Advice function for fixing cc-mode indentation in certain D constructs."
-  (let ((rxp (car args)))
-    (if (and (stringp rxp) (string= rxp "if\\>[^_]"))
-        (or (apply orig-fun '("static\\>\\s-+if\\>[^_]"))
-            (apply orig-fun '("version\\>[^_]"))
-            (apply orig-fun '("debug\\>[^_]"))
-            (apply orig-fun args))
-      (apply orig-fun args))))
-
-(defun d-around--c-add-stmt-syntax (orig-fun &rest args)
-  ;; checkdoc-params: (orig-fun args)
-  "Advice function for fixing cc-mode indentation in certain D constructs."
-  (if (not (string= major-mode "d-mode"))
-      (apply orig-fun args)
-    (progn
-      (add-function :around (symbol-function 'looking-at)
-                    #'d-special-case-looking-at)
-      (unwind-protect
-          (apply orig-fun args)
-        (remove-function (symbol-function 'looking-at)
-                         #'d-special-case-looking-at)))))
+(cc-eval-when-compile
+  (unless (c-safe (c-lang-const c-post-else-no-indent-kwds java))
+
+    (defun d-special-case-looking-at (orig-fun &rest args)
+      ;; checkdoc-params: (orig-fun args)
+      "Advice function for fixing cc-mode indentation in certain D constructs."
+      (let ((rxp (car args)))
+	(if (and (stringp rxp) (string= rxp "if\\>[^_]"))
+            (or (apply orig-fun '("static\\>\\s-+if\\>[^_]"))
+		(apply orig-fun '("version\\>[^_]"))
+		(apply orig-fun '("debug\\>[^_]"))
+		(apply orig-fun args))
+	  (apply orig-fun args))))
+
+    (defun d-around--c-add-stmt-syntax (orig-fun &rest args)
+      ;; checkdoc-params: (orig-fun args)
+      "Advice function for fixing cc-mode indentation in certain D constructs."
+      (if (not (string= major-mode "d-mode"))
+	  (apply orig-fun args)
+	(progn
+	  (add-function :around (symbol-function 'looking-at)
+			#'d-special-case-looking-at)
+	  (unwind-protect
+              (apply orig-fun args)
+            (remove-function (symbol-function 'looking-at)
+                             #'d-special-case-looking-at)))))
 
-(when (version<= "24.4" emacs-version)
-  (advice-add 'c-add-stmt-syntax :around #'d-around--c-add-stmt-syntax))
+    (when (version<= "24.4" emacs-version)
+      (advice-add 'c-add-stmt-syntax :around #'d-around--c-add-stmt-syntax))))
 
 ;;----------------------------------------------------------------------------
 ;;;###autoload


, and my proposed patch for CC Mode.  This patch applies cleanly to the
Emacs 26.3 sources, and all later versions in the Emacs savannah
repository:



diff -r 6a0c16f6ef37 cc-engine.el
--- a/cc-engine.el	Mon Jan 27 17:50:19 2020 +0000
+++ b/cc-engine.el	Sun Feb 02 11:36:01 2020 +0000
@@ -12275,7 +12275,10 @@
 			     (looking-at "else\\>[^_]")
 			     (save-excursion
 			       (goto-char old-pos)
-			       (looking-at "if\\>[^_]")))
+			       (while (looking-at c-post-else-if-prefix-key)
+				 (goto-char (match-end 1))
+				 (c-forward-syntactic-ws))
+			       (looking-at c-post-else-no-indent-key))) ; e.g. "if"
 			;; Special case to avoid deeper and deeper indentation
 			;; of "else if" clauses.
 			)
diff -r 6a0c16f6ef37 cc-langs.el
--- a/cc-langs.el	Mon Jan 27 17:50:19 2020 +0000
+++ b/cc-langs.el	Sun Feb 02 11:36:01 2020 +0000
@@ -2802,6 +2802,34 @@
 		  (c-lang-const c-block-stmt-2-kwds)))))
 (c-lang-defvar c-opt-block-stmt-key (c-lang-const c-opt-block-stmt-key))
 
+(c-lang-defconst c-post-else-if-prefix-kwds
+  "Keywords which can come between \"else\" and \"if\" (or some other member of
+`c-post-else-no-indent-kwds') without giving the substatement being introduced
+extra indentation."
+  ;; This is used in D Mode.
+  t nil)
+
+(c-lang-defconst c-post-else-if-prefix-key
+  ;; Regexp matching a keyword in `c-post-else-if-prefix-kwds', or matching
+  ;; nothing.
+  t (c-make-keywords-re t (c-lang-const c-post-else-if-prefix-kwds)))
+(c-lang-defvar c-post-else-if-prefix-key
+	       (c-lang-const c-post-else-if-prefix-key))
+
+(c-lang-defconst c-post-else-no-indent-kwds
+  "Keywords which when following an \"else\" (possibly with intervening members
+of `c-post-else-if-prefix-kwds') do not cause extra indentation of the
+substatement being introduced."
+  ;; This is used non-trivially in D Mode.
+  t '("if"))
+
+(c-lang-defconst c-post-else-no-indent-key
+  ;; Regexp matching a keyword in `c-post-else-no-indent-kwds', or matching
+  ;; nothing.
+  t (c-make-keywords-re t (c-lang-const c-post-else-no-indent-kwds)))
+(c-lang-defvar c-post-else-no-indent-key
+	       (c-lang-const c-post-else-no-indent-key))
+
 (c-lang-defconst c-simple-stmt-kwds
   "Statement keywords followed by an expression or nothing."
   t    '("break" "continue" "goto" "return")


-- 
Alan Mackenzie (Nuremberg, Germany).

bug#18158: D Mode: Getting rid of the ugly advice on looking-at.

[Vladimir Panteleev]

Hello Alan,

First, thank you very much for reaching out. I held the expectation
that the maintainer(s) of cc-mode would not have any time to spare on
derivative modes distributed outside of Emacs, so your message was a
welcome surprise.

I greatly appreciate you taking the time to prepare a patch for
d-mode. However, I hope you will forgive me if I burden you with a
bigger matter.

As of last year, I had been working on a major update to d-mode. This
new version fixes all known bugs in d-mode filed so far and has many
other improvements. It is located here:

GitHub pull request:
https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/pull/93
Git clone URL: https://github.com/CyberShadow/Emacs-D-Mode.git ("next" branch)
Current version:
https://raw.githubusercontent.com/CyberShadow/Emacs-D-Mode/next/d-mode.el

I'll be happy to create non-GitHub mirrors of the above upon request.

Unfortunately, the only way I found to achieve these improvements in D
language support was through advising even more cc-mode functions,
similar to the "looking-at" advice your message pertains to.

I fully realize that this approach is not very future-proof. I tried
to keep the implementation as unintrusive and unreliant on cc-mode
internals as possible. However, cc-mode is a non-trivially large and
complicated piece of software, and D is a relatively large and
complicated language, so I admit I have likely fallen short of that
goal. My plan for ongoing maintenance was to keep up and address
problems due to changes in cc-mode internals as they occur; however,
considering the amount of time required to debug and devise fixes for
each problem, I may have been overly eager in considering this
approach.
For this reason I had been thinking of contacting you for advice on
how best to proceed.

Some possible ideas I had in mind:

1. Add hooks and variables to cc-mode to allow d-mode to cleanly
configure cc-mode to support the D language.

   Though I believe D firmly fits into the Algol-like family of
languages currently supported by cc-mode, it also has many unique
features, so I worry that this approach would be overly burdensome to
cc-mode.

2. Move d-mode into cc-mode, replacing advice hacks with clean integration.

   I would be happy to contribute my work to GNU and continue
supporting d-mode within cc-mode, if this is a possibility. However,
even though most of the code in my "next" branch was written by me,
the project has been around for a while, and I don't know if we can
find all authors and have them transfer copyright to FSF; or,
alternatively, move only the code I've written, and rewrite the
missing parts, but I don't know how strict we would need to be in this
regard.

3. Reimplement d-mode in cc-mode.

   It might be easier to start from scratch and, using just the
existing d-mode test suite for reference, reimplement d-mode within
cc-mode. Not having to resort to hacks such as advising functions will
likely allow a much cleaner implementation than the current one.

4. Copy relevant parts of cc-mode into d-mode.

   One very ugly, but possibly practical approach would be to include
all relevant cc-mode code in d-mode's distribution. All definitions
would need to be properly namespaced to not conflict with the real
cc-mode. This would effectively "freeze" the cc-mode version that
d-mode uses, thus protecting it from incompatible changes in internals
of future cc-mode versions, though compatibility fixes for future
Emacs versions would still need to be applied, and d-mode will be
unable to benefit from any future cc-mode improvements.

5. Abandon ship, and scale back D language support in d-mode to a
maintainable minimum.

   Perhaps using cc-mode for D language support within Emacs is not an
optimal solution. All of the above ideas would certainly require a
non-trivial amount of further effort to achieve, and the result would
only cover the combination of supporting the D language within Emacs.
An approach that has been recently gaining popularity is, instead of
writing one package for each language for each editor, to use
"language servers" which use a common protocol to provide language
support to any editor. The LSP protocol specification (as maintained
by Microsoft) supports formatting (indentation), outlining (imenu),
and (in recent versions) semantic syntax highlighting (fontification),
with the downside of requiring additional software and configuration.
If there is no interest in supporting D / d-mode in cc-mode (ideas 1-3
above), then perhaps this may be the best course of action.

I would love to hear your thoughts on the above.

Also, looking at the email headers, I see that this message was in
reference to Emacs bug 18158. Looking at the previous messages in the
bug thread (https://debbugs.gnu.org/18158), I found it odd that a bug
in d-mode's behavior was filed at Emacs' bug tracker.

I think that any d-mode or cc-mode issues observed when d-mode is
active ought to be reported to the d-mode bugtracker, and not the
cc-mode one. It is certainly not my intention to burden cc-mode
maintainers with d-mode's bugs or shortcomings. Perhaps there is
something I need to do to make it clearer where to report bugs in
d-mode?

In any case, I've been unable to reproduce the bug (as it was
originally filed, and with the test code provided in message #20
(<8B3ABB9B-7A39-428A-A65B-B77D0614C9FE@gmail.com>), on either the
current master branch or my "next" branch of d-mode. I see that the
bug was filed in 2014. Perhaps it was fixed by this commit in 2015:
https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/commit/27fbe66f6de27f8337fe40d6a19f039c589cd1fc

I added a test case for this fix in this commit:
https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/commit/3e5a5d523d7580b2a42f376cbf83bdd9ac296bbd

Best regards.

bug#18158: D Mode: Getting rid of the ugly advice on looking-at.

[Alan Mackenzie]

Hello, Vladimir,

Thanks for your extensive reply.  I have been thinking it over over the
past few days.

On Sun, Feb 02, 2020 at 16:59:43 +0000, Vladimir Panteleev wrote:
> Hello Alan,

> First, thank you very much for reaching out. I held the expectation
> that the maintainer(s) of cc-mode would not have any time to spare on
> derivative modes distributed outside of Emacs, so your message was a
> welcome surprise.

:-)

> I greatly appreciate you taking the time to prepare a patch for
> d-mode. However, I hope you will forgive me if I burden you with a
> bigger matter.

No problem.

> As of last year, I had been working on a major update to d-mode. This
> new version fixes all known bugs in d-mode filed so far and has many
> other improvements. It is located here:

> GitHub pull request:
> https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/pull/93
> Git clone URL: https://github.com/CyberShadow/Emacs-D-Mode.git ("next" branch)
> Current version:
> https://raw.githubusercontent.com/CyberShadow/Emacs-D-Mode/next/d-mode.el

> I'll be happy to create non-GitHub mirrors of the above upon request.

I think I can get a copy of the repository without having to sign up,
but I haven't had a chance to do this, yet.

> Unfortunately, the only way I found to achieve these improvements in D
> language support was through advising even more cc-mode functions,
> similar to the "looking-at" advice your message pertains to.

Maybe we could work together on this so that these pieces of advice
could be minimised, or even avoided altogether.

Could you possibly give me one or two examples of why these pieces of
advice are needed?  This would give me more of a feel for what the
problems are.

> I fully realize that this approach is not very future-proof. I tried
> to keep the implementation as unintrusive and unreliant on cc-mode
> internals as possible. However, cc-mode is a non-trivially large and
> complicated piece of software, and D is a relatively large and
> complicated language, so I admit I have likely fallen short of that
> goal.

CC Mode is indeed (too) complicated.  Partly this is because the
languages it deals with are complicated (particularly C++), but also, it
has to be said, because the maintainers haven't always adopted an
optimal compromise between speed, accuracy and simplicity.  Accuracy has
tended to be the highest goal, followed by speed.

> My plan for ongoing maintenance was to keep up and address problems
> due to changes in cc-mode internals as they occur; however,
> considering the amount of time required to debug and devise fixes for
> each problem, I may have been overly eager in considering this
> approach.  For this reason I had been thinking of contacting you for
> advice on how best to proceed.

> Some possible ideas I had in mind:

> 1. Add hooks and variables to cc-mode to allow d-mode to cleanly
> configure cc-mode to support the D language.

This would perhaps be the cleanest option.

>    Though I believe D firmly fits into the Algol-like family of
> languages currently supported by cc-mode, it also has many unique
> features, so I worry that this approach would be overly burdensome to
> cc-mode.

How does D compare with C++ in terms of complexity?

One practical step for option 1 would be for you to have write access to
the CC Mode repository, in particular to a "D Mode" branch, where you
could implement the necessary CC Mode features to support D Mode.
Something like this was done ~10 years ago for enhancing Java Mode.

> 2. Move d-mode into cc-mode, replacing advice hacks with clean integration.

I would be less keen on this.  Some while ago, my predecessor Martin
Stjernholm decided as a matter of policy not to have any more languages
directly implemented in CC Mode, and I have continued with this.  The
problem is that if the maintainer of a particular CC Mode mode stops
contributing for any reason, the mode becomes old and fossilised.  This
has already happened to IDL Mode and Pike Mode.

But perhaps a D Mode support file could be included in CC Mode, somewhat
along the lines of how cc-awk.el supports AWK, but including only
essential support for D Mode rather than including D Mode itself.  I'm
not sure if I've explained that very well.

>    I would be happy to contribute my work to GNU and continue
> supporting d-mode within cc-mode, if this is a possibility. However,
> even though most of the code in my "next" branch was written by me,
> the project has been around for a while, and I don't know if we can
> find all authors and have them transfer copyright to FSF; or,
> alternatively, move only the code I've written, and rewrite the
> missing parts, but I don't know how strict we would need to be in this
> regard.

Very strict indeed.  The FSF's policy is that all contributions which
aren't minor (in practice, up to about 15 lines of code) need their
authors to assign their copyright.  I've not known them to deviate from
this policy, which has good reasons, though occasionally it can cause
bad feelings.

> 3. Reimplement d-mode in cc-mode.

>    It might be easier to start from scratch and, using just the
> existing d-mode test suite for reference, reimplement d-mode within
> cc-mode. Not having to resort to hacks such as advising functions will
> likely allow a much cleaner implementation than the current one.

Restarting an existing piece of software from scratch is a risky
process.  One skirts existing bugs cleanly, but there's a high chance of
introducing new bugs which weren't there in the first place.

> 4. Copy relevant parts of cc-mode into d-mode.

>    One very ugly, but possibly practical approach would be to include
> all relevant cc-mode code in d-mode's distribution. All definitions
> would need to be properly namespaced to not conflict with the real
> cc-mode. This would effectively "freeze" the cc-mode version that
> d-mode uses, thus protecting it from incompatible changes in internals
> of future cc-mode versions, though compatibility fixes for future
> Emacs versions would still need to be applied, and d-mode will be
> unable to benefit from any future cc-mode improvements.

I don't think this is the best way forward.  Even though it could work,
it's likely to involve a lot of unrewarding work, and some duplication
of effort in fixing bugs common to CC Mode and D Mode.

> 5. Abandon ship, and scale back D language support in d-mode to a
> maintainable minimum.

This would be entirely up to you, but I suspect that D Mode users would
be unhappy with losing useful features for the sake of maintainability.

>    Perhaps using cc-mode for D language support within Emacs is not an
> optimal solution. All of the above ideas would certainly require a
> non-trivial amount of further effort to achieve, and the result would
> only cover the combination of supporting the D language within Emacs.
> An approach that has been recently gaining popularity is, instead of
> writing one package for each language for each editor, to use
> "language servers" which use a common protocol to provide language
> support to any editor. The LSP protocol specification (as maintained
> by Microsoft) supports formatting (indentation), outlining (imenu),
> and (in recent versions) semantic syntax highlighting (fontification),
> with the downside of requiring additional software and configuration.
> If there is no interest in supporting D / d-mode in cc-mode (ideas 1-3
> above), then perhaps this may be the best course of action.

This is a big topic indeed.  There have been a few discussions around
LSP on the Emacs developer mailing list in the past few months.  Using
LSP in place of the current mechanisms would involve a lot of work, but
would likely be worth it.  Recently, somebody has implemented
fontification for Java Mode using Emacs's own analysis suite (whose name
I can't recall at the moment).  So it seems things are moving in that
direction.

Personally, I have mixed feelings about LSP.  It may be a better way of
doing things, but comes along at a time when traditional CC Mode is very
highly developed over several decades of hard work.  It's a bit like
when vinyl records were superseded by CDs just around the time when we'd
learnt how to make high quality records.  For a while, the new CDs
weren't as good.

> I would love to hear your thoughts on the above.

I hope I've given you enought to think about.  ;-)

> Also, looking at the email headers, I see that this message was in
> reference to Emacs bug 18158. Looking at the previous messages in the
> bug thread (https://debbugs.gnu.org/18158), I found it odd that a bug
> in d-mode's behavior was filed at Emacs' bug tracker.

It's bound to happen every now and then.  I actually missed this bug
back in 2014 and wasn't aware of it until Stefan Kangas, who's been
going through hundreds of open old bugs trying to settle them, alerted
me to it.

> I think that any d-mode or cc-mode issues observed when d-mode is
> active ought to be reported to the d-mode bugtracker, and not the
> cc-mode one. It is certainly not my intention to burden cc-mode
> maintainers with d-mode's bugs or shortcomings. Perhaps there is
> something I need to do to make it clearer where to report bugs in
> d-mode?

I don't think you need to worry too much, very few D Mode bugs get sent
to the CC Mode or the Emacs mailing lists.

> In any case, I've been unable to reproduce the bug (as it was
> originally filed, and with the test code provided in message #20
> (<8B3ABB9B-7A39-428A-A65B-B77D0614C9FE@gmail.com>), on either the
> current master branch or my "next" branch of d-mode. I see that the
> bug was filed in 2014. Perhaps it was fixed by this commit in 2015:
> https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/commit/27fbe66f6de27f8337fe40d6a19f039c589cd1fc

> I added a test case for this fix in this commit:
> https://github.com/Emacs-D-Mode-Maintainers/Emacs-D-Mode/commit/3e5a5d523d7580b2a42f376cbf83bdd9ac296bbd

To reproduce it, I temporarily removed the advice on `looking-at' from D
Mode.  The problem was then apparent in the test code supplied by Liran
Zvibel on 28th January to me, Cc: to the Emacs bug list.

> Best regards.

-- 
Alan Mackenzie (Nuremberg, Germany).

bug#18158: D Mode: Getting rid of the ugly advice on looking-at.

[Vladimir Panteleev]

Hi Alan,

On Fri, 7 Feb 2020 at 21:31, Alan Mackenzie <acm@muc.de> wrote:> >
Unfortunately, the only way I found to achieve these improvements in D
> Could you possibly give me one or two examples of why these pieces of
> advice are needed?  This would give me more of a feel for what the
> problems are.

Yes, gladly.

d-mode installs advice in seven^Wsix places, all of which are in
cc-mode. Here is a rough / brief description of each.

1. The looking-at call from within c-add-stmt-syntax which started
this discussion.

   It has been later extended in
075c3e7b8da8bfffe1bbe00cf705494755959fd1 to also apply to "version"
and "debug" blocks.

2. A replacement of c-forward-decl-or-cast-1.

   The D version, which is an edited copy of the cc-mode function, has
additional code required to recognize and fontify variable
declarations in foreach statements and lambda expressions, as well as
conditional compilation. The D version (d-forward-decl-or-cast-1)
completely replaces c-forward-decl-or-cast-1, and only a small part of
the original remains.

3. A replacement of c-get-fontification-context.

   This one is also used for foreach loops and lambda parameter lists.
I'm not sure the "integration" with cc-mode is that great here - the
code might be using some constants with different conventions than
cc-mode.

4. A replacement of c-in-knr-argdecl.

   Here d-mode abuses cc-mode's logic for two features that are
syntactically a bit similar to K&R C argument lists - function
contracts and template constraints. They also exist between the
function signature and its body, so making use of the existing K&R
mechanics in cc-mode seemed like the simplest way to fix support for
these constructs.

5. A replacement of c-forward-type.

   This one also implements only exactly the syntax in the D
programming language.

6. A modified version of c-update-brace-stack.

   The modifications were necessary to help cc-mode understand static
compilation constructs in the D language. Syntax such as "static if"
does not change whether its substatements are "top-level" or not, and
it can be used with or without a { } pair, which required custom
logic.

As I was making this list, I couldn't remember what one of the advice
was for, so I removed it - and the test suite still passed, so it
seems like my newer changes made it obsolete. I committed the removal.

> How does D compare with C++ in terms of complexity?

I would say D is definitely less complicated than C++, and it's
especially easier to parse (it was designed from the beginning to
avoid pitfalls such as the ambiguity of "a < b, c > d"). However, it
also has many features (for instance, depending on how you count, it
has about seven kinds of string literals). Some troublesome aspects I
ran into while working on d-mode were type inference (types may
sometimes be omitted from declarations) and parameter lists ("(a, b,
c)" is a list of types for a function declaration, like in C, but a
parameter name list with inferred types for lambdas).

> One practical step for option 1 would be for you to have write access to
> the CC Mode repository, in particular to a "D Mode" branch, where you
> could implement the necessary CC Mode features to support D Mode.
> Something like this was done ~10 years ago for enhancing Java Mode.

That would work; though, I might need a bit of guidance with making my
cc-mode modifications acceptable :) E.g., what would be a good way for
derived modes to replace cc-mode functions such as c-forward-type.

> But perhaps a D Mode support file could be included in CC Mode, somewhat
> along the lines of how cc-awk.el supports AWK, but including only
> essential support for D Mode rather than including D Mode itself.  I'm
> not sure if I've explained that very well.

I think I understand. Probably it will be clearer which way is best
once we attempt to do so.

> > I would love to hear your thoughts on the above.
>
> I hope I've given you enought to think about.  ;-)

Indeed. Thank you for the extensive reply. I agree with and have no
further comments on everything you said not quoted above.

> To reproduce it, I temporarily removed the advice on `looking-at' from D
> Mode.  The problem was then apparent in the test code supplied by Liran
> Zvibel on 28th January to me, Cc: to the Emacs bug list.

Oh, I see. Thank you for the clarification. I guess I could have put
two and two together :)

Best regards.