msteveb / jimtcl

Official repository of Jim Tcl, an open-source, small footprint implementation of Tcl
http://jim.tcl.tk/
Other
445 stars 123 forks source link

Backwards compatibility for oo.tcl #293

Open prpr19xx opened 9 months ago

prpr19xx commented 9 months ago

Following on from #248...

Before 5af9d6a, this worked without complaint:

. package require oo
1.0
. class t {v1 0 v2 0}
t
. set v [t new {v1 1 v3 3}]
::<reference.<t______>.00000000000000000002>
. $v get v1
1
. $v get v2
0

Since then, it generates an error: t defaultconstructor, v3 is not a class variable

I don't care about v3, I just don't want it to cause any errors (this is a much simplified test-case whereas in reality there are many variables I'm not interested in, but it's huge work to sort in our existing code). As a workaround, I have been manually patching out the "return -code error..." statement that causes it to fail, restoring previous behaviour. But I'd rather not have to patch at all, and it would seem that this is fairly easily fixed by adding something like the following change to allow selectable behaviour:

diff --git a/oo.tcl b/oo.tcl
index 4fefa9c..96fca14 100644
--- a/oo.tcl
+++ b/oo.tcl
@@ -1,4 +1,5 @@
 # OO support for Jim Tcl, with multiple inheritance
+set ::oo::ignore_invalid_vars false

 # Create a new class $classname, with the given
 # dictionary as class variables. These are the initial
@@ -81,11 +82,14 @@ proc class {classname {baseclasses {}} classvars} {
        $classname method defaultconstructor {{__vars {}}} {
                set __classvars [$self classvars]
                foreach __v [dict keys $__vars] {
-                       if {![dict exists $__classvars $__v]} {
-                               # level 3 because defaultconstructor is called by new
-                               return -code error -level 3 "[lindex [info level 0] 0], $__v is not a class variable"
+                       if {[dict exists $__classvars $__v]} {
+                               set $__v [dict get $__vars $__v]
+                       } else {
+                               if {!$::oo::ignore_invalid_vars} {
+                                       # level 3 because defaultconstructor is called by new
+                                       return -code error -level 3 "[lindex [info level 0] 0], $__v is not a class variable"
+                               }
                        }
-                       set $__v [dict get $__vars $__v]
                }
        }
        alias "$classname constructor" "$classname defaultconstructor"

This maintains the current behaviour, but allows regression to the previous e.g.

. package require oo
1.0
. puts $::oo::ignore_invalid_vars
false
. class t {v1 0 v2 0}
t
. set v [t new {v1 1 v3 3}]
t defaultconstructor, v3 is not a class variable
[error] . 
. set ::oo::ignore_invalid_vars true
true
. set v [t new {v1 1 v3 3}]
::<reference.<t______>.00000000000000000002>
. $v get v1
1
. $v get v2
0

There may be a better way, but this works for me.

msteveb commented 9 months ago

I'll have a think about this. I'm probably not inclined to bake in backward compatibility, but I'll consider an approach that will make it easy for you to do this in your own code without modifying oo.tcl

msteveb commented 1 month ago

So can't you simply create your own constructor that behaves as you want rather than using the default constructor?

e.g.

t method constructor {arglist} {
    foreach {k v} $arglist {
        set $k $v
    }
}

Just need to do this for each class you create where you want to use the previous behaviour.

The-Markitecht commented 1 month ago

... and that also is easily automated for sugar:

# untested example:
proc makeCompatible {className} {
    $className method constructor {arglist} {
    foreach {k v} $arglist {
        set $k $v
    }
}

class t {v1 0 v2 0}
makeCompatible t

You could even sugar it further than that, pretty easily, by rolling that functionality into your own wrapper of the class command, call it class, after rename'ing the original one.