unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Compatibility patches for EDE / EIEIO
@ 2021-01-10 16:52 Eric Ludlam
  2021-01-11 21:21 ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Ludlam @ 2021-01-10 16:52 UTC (permalink / raw)
  To: emacs-devel

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

Hi,

Attached are two proposed patches related to compatibility for EIEIO and 
EDE.

The EIEIO patch helps with loading old EDE save files.  It updates 
existing compatibility for the 1st object name argument to constructor 
during object loading from a file.  In older versions of EIEIO, objects 
were saved with a 'name'.  New EIEIO skips the name on load.  This patch 
will apply the name to the :object-name slot if it is a subclass of 
eieio-named.  This will compatibly bring in old objects and maintain the 
name.

I tried it with the existing eieio tests which passed.

The EDE patch brings over a change from the CEDET repository on Source 
Forge that enables project types detected via a function.  The function 
based project types were never integrated into Emacs.  This patch allows 
these old project types from SourceForge to work with built-in EDE. 
There are no uses of this feature in Emacs, but it is useful for 
integrating new project types that are not easily identified by a 
string, such as android, arduino, fitbit and other tools that usually 
provide their own 'studio' thing.

The EDE patch was written by me before the last 'release' I had signed 
from my company.  The EIEIO patch is new.

Thanks
Eric

[-- Attachment #2: 0001-cedet-ede-auto.el.patch --]
[-- Type: text/x-patch, Size: 1945 bytes --]

From c2e4ef56934fc08746cb8fb629f67b01fbd8a734 Mon Sep 17 00:00:00 2001
From: Eric Ludlam <zappo@gnu.org>
Date: Sun, 10 Jan 2021 10:37:50 -0500
Subject: [PATCH 1/2] cedet/ede/auto.el:

(ede-calc-fromconfig): New method.  Support functions in addition to
string matchers.
(ede-dirmatch-installed, ede-do-dirmatch):
Use `ede-calc-fromconfig' to do conversion.
Author: Eric Ludlam <zappo@gnu.org>
---
 lisp/cedet/ede/auto.el | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/lisp/cedet/ede/auto.el b/lisp/cedet/ede/auto.el
index ee75e29799..e1417d7806 100644
--- a/lisp/cedet/ede/auto.el
+++ b/lisp/cedet/ede/auto.el
@@ -64,24 +64,22 @@ ede-project-autoload-dirmatch
 can be used to define that match without loading the specific project
 into memory.")
 
+(cl-defmethod ede-calc-fromconfig ((dirmatch ede-project-autoload-dirmatch))
+  "Calculate the value of :fromconfig from DIRMATCH."
+  (let* ((fc (oref dirmatch fromconfig))
+	 (found (cond ((stringp fc) fc)
+		      ((functionp fc) (funcall fc))
+		      (t (error "Unknown dirmatch object match style.")))))
+    (expand-file-name found)
+    ))
+
 (cl-defmethod ede-dirmatch-installed ((dirmatch ede-project-autoload-dirmatch))
   "Return non-nil if the tool DIRMATCH might match is installed on the system."
-  (let ((fc (oref dirmatch fromconfig)))
-
-    (cond
-     ;; If the thing to match is stored in a config file.
-     ((stringp fc)
-      (file-exists-p fc))
-
-     ;; Add new types of dirmatches here.
-
-     ;; Error for weird stuff
-     (t (error "Unknown dirmatch type.")))))
-
+  (file-exists-p (ede-calc-fromconfig dirmatch)))
 
 (cl-defmethod ede-do-dirmatch ((dirmatch ede-project-autoload-dirmatch) file)
   "Does DIRMATCH match the filename FILE."
-  (let ((fc (oref dirmatch fromconfig)))
+  (let ((fc (ede-calc-fromconfig dirmatch)))
 
     (cond
      ;; If the thing to match is stored in a config file.
-- 
2.25.1


[-- Attachment #3: 0002-eieio-base.el.patch --]
[-- Type: text/x-patch, Size: 2498 bytes --]

From f15b0f11c0fb9689454d687e49c1d7edbc26f0fc Mon Sep 17 00:00:00 2001
From: Eric Ludlam <zappo@gnu.org>
Date: Sun, 10 Jan 2021 10:54:49 -0500
Subject: [PATCH 2/2] eieio-base.el:

(eieio-persistent-make-instance): Save the backward compatible 'name'
of objects saved in the file, and if the newly loaded class inherits
from 'eieio-named', restore the name of the object.
Author: Eric Ludlam <zappo@gnu.org>
---
 lisp/emacs-lisp/eieio-base.el | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/lisp/emacs-lisp/eieio-base.el b/lisp/emacs-lisp/eieio-base.el
index 4ba72aea56..19809265ff 100644
--- a/lisp/emacs-lisp/eieio-base.el
+++ b/lisp/emacs-lisp/eieio-base.el
@@ -264,12 +264,17 @@ eieio-persistent-make-instance
   (:method
    ((objclass (subclass eieio-default-superclass)) inputlist)
 
-   (let ((slots (if (stringp (car inputlist))
-                    ;; Earlier versions of `object-write' added a
-                    ;; string name for the object, now obsolete.
-                    (cdr inputlist)
-                  inputlist))
-         (createslots nil))
+   (let* ((name nil)
+          (slots (if (stringp (car inputlist))
+                     (progn
+                       ;; Earlier versions of `object-write' added a
+                       ;; string name for the object, now obsolete.
+                       ;; Save as 'name' in case this object is subclass
+                       ;; of eieio-named with no :object-name slot specified.
+                       (setq name (car inputlist))
+                       (cdr inputlist))
+                   inputlist))
+          (createslots nil))
      ;; If OBJCLASS is an eieio autoload object, then we need to
      ;; load it (we don't need the return value).
      (eieio--full-class-object objclass)
@@ -286,7 +291,17 @@ eieio-persistent-make-instance
 
        (setq slots (cdr (cdr slots))))
 
-     (apply #'make-instance objclass (nreverse createslots)))))
+     (let ((newobj (apply #'make-instance objclass (nreverse createslots))))
+
+       ;; Check for special case of subclass of `eieio-named', and do
+       ;; name assignment.
+       (when (and eieio-backward-compatibility
+                  (object-of-class-p newobj eieio-named)
+                  (not (oref newobj object-name))
+                  name)
+         (oset newobj object-name name))
+
+       newobj))))
 
 (defun eieio-persistent-fix-value (proposed-value)
   "Fix PROPOSED-VALUE.
-- 
2.25.1


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

* Re: Compatibility patches for EDE / EIEIO
  2021-01-10 16:52 Compatibility patches for EDE / EIEIO Eric Ludlam
@ 2021-01-11 21:21 ` Stefan Monnier
  2021-01-17 19:57   ` Eric Ludlam
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2021-01-11 21:21 UTC (permalink / raw)
  To: Eric Ludlam; +Cc: emacs-devel

> Attached are two proposed patches related to compatibility for EIEIO
>  and EDE.

Thanks, pushed both to `master`,


        Stefan




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

* Re: Compatibility patches for EDE / EIEIO
  2021-01-11 21:21 ` Stefan Monnier
@ 2021-01-17 19:57   ` Eric Ludlam
  2021-01-17 22:16     ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Ludlam @ 2021-01-17 19:57 UTC (permalink / raw)
  To: Stefan Monnier, Eric Ludlam; +Cc: emacs-devel

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

On 1/11/21 4:21 PM, Stefan Monnier wrote:
>> Attached are two proposed patches related to compatibility for EIEIO
>>   and EDE.
> Thanks, pushed both to `master`,
Attached is one more small patch I had missed in my previous set. This 
change is independent of the other two.  This is also needed to load 
Project.ede files that happen to include custom rules, where those rules 
are also objects.  The Project loader's error checking in eieio-base.el 
requires the :type to be precise when the value can be a list of objects.

I used the same basic mechanism used in ede/base.el for ede-project's 
'targets' slot.

If you want to verify the changes, this project uses the feature:

https://sourceforge.net/p/matlab-emacs/src/ci/master/tree/

Use global-ede-mode and visit this project.

Thanks
Eric

[-- Attachment #2: 0003-lisp-cedet-ede-proj.el.patch --]
[-- Type: text/x-patch, Size: 1247 bytes --]

From d07f0c4711fa8f82c4415a2a69d0338006088205 Mon Sep 17 00:00:00 2001
From: Eric Ludlam <zappo@gnu.org>
Date: Sun, 17 Jan 2021 14:41:51 -0500
Subject: [PATCH 3/3] lisp/cedet/ede/proj.el:

(ede-makefile-rule-list): New type.
(ede-proj-target-makefile): Set :type of :rules slot to
ede-makefile-rule-list.  This enables Project files to
load.
Author: Eric Ludlam <zappo@gnu.org>
---
 lisp/cedet/ede/proj.el | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/cedet/ede/proj.el b/lisp/cedet/ede/proj.el
index 59628ebf4c..78b852378c 100644
--- a/lisp/cedet/ede/proj.el
+++ b/lisp/cedet/ede/proj.el
@@ -150,6 +150,9 @@ ede-proj-target
    )
   "Abstract class for ede-proj targets.")
 
+(unless (fboundp 'ede-makefile-rule-p)
+  (cl-deftype ede-makefile-rule-list () '(list-of ede-makefile-rule)))
+
 (defclass ede-proj-target-makefile (ede-proj-target)
   ((makefile :initarg :makefile
 	     :initform "Makefile"
@@ -184,7 +187,7 @@ ede-proj-target-makefile
 commands where the variable would usually appear.")
    (rules :initarg :rules
 	  :initform nil
-	  :type list
+	  :type ede-makefile-rule-list
 	  :custom (repeat (object :objecttype ede-makefile-rule))
 	  :label "Additional Rules"
 	  :group (make)
-- 
2.25.1


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

* Re: Compatibility patches for EDE / EIEIO
  2021-01-17 19:57   ` Eric Ludlam
@ 2021-01-17 22:16     ` Stefan Monnier
  2021-01-18 16:52       ` Eric Ludlam
  0 siblings, 1 reply; 6+ messages in thread
From: Stefan Monnier @ 2021-01-17 22:16 UTC (permalink / raw)
  To: Eric Ludlam; +Cc: emacs-devel, Eric Ludlam

Hi Eric,

> +(unless (fboundp 'ede-makefile-rule-p)
> +  (cl-deftype ede-makefile-rule-list () '(list-of ede-makefile-rule)))

I don't understand the (fboundp 'ede-makefile-rule-p) test, maybe it
deserves a comment?

>  (defclass ede-proj-target-makefile (ede-proj-target)
>    ((makefile :initarg :makefile
>  	     :initform "Makefile"
> @@ -184,7 +187,7 @@ ede-proj-target-makefile
>  commands where the variable would usually appear.")
>     (rules :initarg :rules
>  	  :initform nil
> -	  :type list
> +	  :type ede-makefile-rule-list

How 'bout using

	  :type (list-of ede-makefile-rule)

?


        Stefan




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

* Re: Compatibility patches for EDE / EIEIO
  2021-01-17 22:16     ` Stefan Monnier
@ 2021-01-18 16:52       ` Eric Ludlam
  2021-01-18 17:52         ` Stefan Monnier
  0 siblings, 1 reply; 6+ messages in thread
From: Eric Ludlam @ 2021-01-18 16:52 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel, Eric Ludlam

Thanks for looking at my patch,

The short answer to your questions is that I'm not very familiar with 
how the typing system works with the latest eieio changes.  I'm fine 
with the proposed use of (list-of ...) as it seems much simpler and more 
obvious.

I would expect you would want to make the same change in ede/base.el 
around line 172, which is the code I used as a template to make :rules 
work.   Maybe something like the below patch?

I suspect the fboundp part was transitional when the old auto-generated 
predicates were deprecated. Maybe useful in an external bit of elisp 
that needs to support multiple versions, but not here.  Thanks for 
pointing this out.

Eric

diff --git a/lisp/cedet/ede/base.el b/lisp/cedet/ede/base.el
index 7799746e0c..b26984712a 100644
--- a/lisp/cedet/ede/base.el
+++ b/lisp/cedet/ede/base.el
@@ -160,8 +160,6 @@ ede-project-placeholder
  ;; Projects can also affect how EDE works, by changing what appears in
  ;; the EDE menu, or how some keys are bound.
  ;;
-(unless (fboundp 'ede-target-list-p)
-  (cl-deftype ede-target-list () '(list-of ede-target)))

  (defclass ede-project (ede-project-placeholder)
    ((subproj :initform nil
@@ -169,7 +167,7 @@ ede-project
          :documentation "Sub projects controlled by this project.
  For Automake based projects, each directory is treated as a project.")
     (targets :initarg :targets
-        :type ede-target-list
+        :type (list-of ede-target)
          :custom (repeat (object :objectcreatefcn ede-new-target-custom))
          :label "Local Targets"
          :group (targets)

On 1/17/21 5:16 PM, Stefan Monnier wrote:
> Hi Eric,
>
>> +(unless (fboundp 'ede-makefile-rule-p)
>> +  (cl-deftype ede-makefile-rule-list () '(list-of ede-makefile-rule)))
> I don't understand the (fboundp 'ede-makefile-rule-p) test, maybe it
> deserves a comment?
>
>>   (defclass ede-proj-target-makefile (ede-proj-target)
>>     ((makefile :initarg :makefile
>>   	     :initform "Makefile"
>> @@ -184,7 +187,7 @@ ede-proj-target-makefile
>>   commands where the variable would usually appear.")
>>      (rules :initarg :rules
>>   	  :initform nil
>> -	  :type list
>> +	  :type ede-makefile-rule-list
> How 'bout using
>
> 	  :type (list-of ede-makefile-rule)
>
> ?
>
>
>          Stefan
>




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

* Re: Compatibility patches for EDE / EIEIO
  2021-01-18 16:52       ` Eric Ludlam
@ 2021-01-18 17:52         ` Stefan Monnier
  0 siblings, 0 replies; 6+ messages in thread
From: Stefan Monnier @ 2021-01-18 17:52 UTC (permalink / raw)
  To: Eric Ludlam; +Cc: emacs-devel, Eric Ludlam

> The short answer to your questions is that I'm not very familiar with how
> the typing system works with the latest eieio changes.

I suspect we're the two most knowledgeable about it, so if you have
a doubt about one part I hope I can help.

> I'm fine with the proposed use of (list-of ...) as it seems much
> simpler and more obvious.

OK, I've changed the code to use that.

> I would expect you would want to make the same change in ede/base.el around
> line 172, which is the code I used as a template to make :rules work.  
> Maybe something like the below patch?

Looks good to me.

So, I pushed the patch below,


        Stefan


diff --git a/lisp/cedet/ede/base.el b/lisp/cedet/ede/base.el
index 7799746e0c..810d6ef3bd 100644
--- a/lisp/cedet/ede/base.el
+++ b/lisp/cedet/ede/base.el
@@ -160,16 +160,13 @@ ede-project-placeholder
 ;; Projects can also affect how EDE works, by changing what appears in
 ;; the EDE menu, or how some keys are bound.
 ;;
-(unless (fboundp 'ede-target-list-p)
-  (cl-deftype ede-target-list () '(list-of ede-target)))
-
 (defclass ede-project (ede-project-placeholder)
   ((subproj :initform nil
 	    :type list
 	    :documentation "Sub projects controlled by this project.
 For Automake based projects, each directory is treated as a project.")
    (targets :initarg :targets
-	    :type ede-target-list
+	    :type (list-of ede-target)
 	    :custom (repeat (object :objectcreatefcn ede-new-target-custom))
 	    :label "Local Targets"
 	    :group (targets)
diff --git a/lisp/cedet/ede/proj.el b/lisp/cedet/ede/proj.el
index 59628ebf4c..4af8b4104f 100644
--- a/lisp/cedet/ede/proj.el
+++ b/lisp/cedet/ede/proj.el
@@ -184,7 +184,7 @@ ede-proj-target-makefile
 commands where the variable would usually appear.")
    (rules :initarg :rules
 	  :initform nil
-	  :type list
+	  :type (list-of ede-makefile-rule)
 	  :custom (repeat (object :objecttype ede-makefile-rule))
 	  :label "Additional Rules"
 	  :group (make)




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

end of thread, other threads:[~2021-01-18 17:52 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10 16:52 Compatibility patches for EDE / EIEIO Eric Ludlam
2021-01-11 21:21 ` Stefan Monnier
2021-01-17 19:57   ` Eric Ludlam
2021-01-17 22:16     ` Stefan Monnier
2021-01-18 16:52       ` Eric Ludlam
2021-01-18 17:52         ` Stefan Monnier

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