|
From: Emiliano <emi...@gm...> - 2025-09-23 01:06:16
|
On Mon, 22 Sep 2025 22:18:13 +0200 Csaba Nemethi <csa...@t-...> wrote: > Hi Emiliano, > > Many thanks for your comments! See my answer embedded into your text. Many thanks for taking the time to read them, and for moving this forward! See inline comments ... > > It is common practice in Tk that commands by name are created in the > global namespace. Example: image create <type> <name> ... Here the > <name> command is always created in the global namespace. Why should we > break this rule? Tk predates namespaces, so compatibility plays an important role here. But this is not the case with this functionality, which is meant to be used primarily by package developers, so they are expected to put all their data inside a namespace. That said, even the current docs recommends, in case of images, to use namespaces. image(n) reads "It is important to note that the image command will silently overwrite any procedure that may currently be defined by the given name, so choose the name wisely. It is recommended to use a separate namespace for image names (e.g., ::img::logo, ::img::large)." I'll argue that, while *named* images should be created from the global scope (so they are easier to find), non-named images, as created by [image create $type -opt ...] should be created in the current namespace as the returned name will be assigned to a variable anyway. Just as [oo::object new] does. > The implementation already makes sure that an already existing tableName > command will be overwritten. This is explicitly mentioned in the manual: > > "If an attribute table of the given name already exists then it is > replaced with the new one and all the attributes of all widgets set > using the old table instance will be unset." Sorry, didn't read the docs, just the tip. My bad! > Also, in all the tableName instance subcommands, if pathName doesn't > exist then the implementation already raises an error. This is valid > for all op names (set|get|names|unset|clear|exists). Why should the > "exists" case be handled differently? For the same reason [winfo] returns an error when asked for info about a non-existing widget but doesn't when asked about whether it [winfo exists]. It should return just true or false. > > * tableName set > > > > Preference to return the full key-value pair for pathName in > > tableName, just as [dict set] does. > > > > What would this be good for? Do you have a convincing use case? Would > it have any benefit over retrieving this information via "tableName get" ? No. As said, just consistency with [dict set]. While I like consistency, this can be left out. Fine with me. > > * tableName names > > > > Preferred API > > > > ** tableName names ?pathName? > > > > returning the list of path names in tableName if pathName is not > > specified. Otherwise, it returns the list of keys already set for > > pathName in tableName. > > > > Here, the word "names" always refers to attribute names, aka keys. Your > proposal would add a second meaning to it. What about a new subcommand > "tableName pathnames" instead? Also fine with me. This is also for the sake of consistency. When some functionality in Tcl or Tk uses a handle or any kind of mapping, the "names" subcommand (array names, image names ...) usually is a list of keys. In this case, we have two cascade mappings in play: window -> attributes dictionary and attribute (or key, or name ... ) -> value. I simply thought in collapsing both in a single command. > > About the implementation: is there any technical advantage to do this > > in Tcl and not C? > > > > The implementation in Tcl is much simpler and easier to maintain. > Currently it has no more than 228 LOC, of which 81 are comments or empty > lines. Sorry, but I beg to differ on this point. A C implementation is not much longer. tkcargo is 466 lines of C code and it actually have more functionality than the one proposed here, using standard Tcl and Tk C API. It is also way faster, mainly when the number of widgets and the number of tables become quite large [1]. I've attached four files, three measuring test scripts and a result csv file. The three test scripts are almost the same, one running with plain Tk (not tables) to be the reference value, one with [tk attribtable] and one with tkcargo. They are different as I just want to be sure I was testing the right thing and be sure I didn't srew up things. While the results seem somewhat noisy, the tendency is clear. Also, I'm not comfortable with the modification of Tk_DestroyWindow to support this use case, based on two points. First, the cleanup script will be forced to run even for widgets that are not in any table, a source of slowdown. And the other is a matter of style: when introducing any other functionality which needs to perform some cleanup when a widget is destroyed, will we keep adding scripts to run? Regards Emiliano [1] Result also show something that looks like O(n^2) when detroying large number of widgets, for example a frame with several thousand children. But this has to be investigated independently of the ongoing discussion. |