Conversation
|
|
||
| #if TCL_MAJOR_VERSION >= 8 | ||
| #if TCL_MAJOR_VERSION >= 9 | ||
| # define CONST const |
There was a problem hiding this comment.
https://wiki.tcl-lang.org/page/Porting+extensions+to+Tcl+9
Changed CONST to const
Changed CONST84 to const
Changed CONST86 to const
This preserves previous behavior, I believe (have not tested with tcl 8, tho)
| #if TCL_MAJOR_VERSION >= 9 | ||
| # define TYPE_TCL_SIZE Tcl_Size | ||
| #else | ||
| # define TYPE_TCL_SIZE int | ||
| #endif |
There was a problem hiding this comment.
https://wiki.tcl-lang.org/page/Porting+extensions+to+Tcl+9
Replaced ìnt with Tcl_Size in functions having Tcl_Size as output parameter
Basically, whenever the code was trying to use a Tcl_Size data, previously it was an int type. So this should work, keeping previous behavior
| struct th_vwait_param *param = (struct th_vwait_param *) clientData; | ||
|
|
||
| if (flags & (TCL_INTERP_DESTROYED | TCL_TRACE_DESTROYED)) { | ||
| if ((flags & TCL_TRACE_DESTROYED) || Tcl_InterpDeleted(interp)) { |
There was a problem hiding this comment.
https://wiki.tcl-lang.org/page/Porting+extensions+to+Tcl+9
Removed expressions using obsolete TCL_INTERP_DESTROYED
Using the suggested Tcl_InterpDeleted instead
I have not tested it thoroughly, just a superficial "does it run the first sample"
| #else | ||
| # define TCL_RELEASE_FUNCTION Tk_Release | ||
| #endif | ||
| TCL_RELEASE_FUNCTION((ClientData)mainWin); |
There was a problem hiding this comment.
https://wiki.tcl-lang.org/page/Porting+extensions+to+Tcl+9
Replaced Tk_Release with Tcl_Release
Changed the function being called depending on the version
| #else | ||
| # define TCL_PRESERVE_FUNCTION Tk_Preserve | ||
| #endif | ||
| TCL_PRESERVE_FUNCTION((ClientData)mainWin); |
There was a problem hiding this comment.
https://wiki.tcl-lang.org/page/Porting+extensions+to+Tcl+9
Replaced Tk_Preserve with Tcl_Preserve
Changed the function being called depending on the version
| } | ||
| /* Tcl_MakeSafe was removed, how to indicate this? */ | ||
|
|
||
| /* if (Tcl_MakeSafe(ptr->ip) == TCL_ERROR) { |
There was a problem hiding this comment.
https://core.tcl-lang.org/tips/doc/trunk/tip/624.md
Tcl_MakeSafe was removed as it not worked as intended.
For now, I just removed this piece of native code, but I think that perhaps the ruby code should be removed too? Or perhaps log a warning?
|
Thank you for working on this. The changes look good to me, and it still builds locally with Tcl/Tk 8.6. Unfortunately, it looks like my maintainer access to this repository has been removed, so I can't currently approve the CI run or merge this. @hsbt I'm guessing this is related to the recent access changes, but when you have time, could you please renable my maintainer access to this repository. |
|
Thanks, @jeremyevans 🤩 I have been struggling for about 9 months with ruby tcl/tk, so just knowing that there is hope to be fixed gives my heart warmth. Also, this is my first time with ruby native extensions. I am not comfortable with what I have done in the |
I've only reviewed the code briefly, but I think a better approach may be adjusting |
Regex mathinc primarily |
ext/tk/extconf.rb
Outdated
| elsif tkconf | ||
| tk_glob = "*tk#{stub}#{tkconf['TK_MAJOR_VERSION']}{.,}#{tkconf['TK_MINOR_VERSION']}*.*" | ||
| tk_regexp = /^.*(tk#{stub}#{tkconf['TK_MAJOR_VERSION']}(?:\.|)#{tkconf['TK_MINOR_VERSION']}.*)\.#{exts}.*$/ | ||
| tk_regexp = /^(?:.*(tcl#{tclconf['TCL_MAJOR_VERSION']})|.*)(tk#{stub}#{tkconf['TK_MAJOR_VERSION']}(?:\.|)#{tkconf['TK_MINOR_VERSION']}.*)\.#{exts}.*$/ |
There was a problem hiding this comment.
This probably works, but seems a bit weird. To be fair, the existing regexp looks odd too. Why start with ^.* and end with .*$? Seems like you could just leave those out. Why (?:\.|) instead of just \.??
I think the extra grouping can be avoided, as the tcl#{tclconf['TCL_MAJOR_VERSION']} could be in optional uncaptured group at the beginning of the first captured group.
Does this work?
tk_regexp = /((?:tcl#{tclconf['TCL_MAJOR_VERSION']})?tk#{stub}#{tkconf['TK_MAJOR_VERSION']}\.?#{tkconf['TK_MINOR_VERSION']}.*)\.#{exts}/
There was a problem hiding this comment.
It worked way better!
Avoiding the ^.* was the key step to just use the (?:tcl#{tclconf['TCL_MAJOR_VERSION'})? in optional group. Trying to keep the old regex led to the abomination.
I'll also simplify tcl_regexp above.
Thanks @jeremyevans!
It seems that `try_func` is indeed generating the right code
2 participants
Fix #59