Tcl Source Code

Tcl_LinkVar is not tolerant to minus, plus, dot

This was reported privately first and published on Tcl-Core list later. The originally problem reported by a user is that typing a bare minus sign ('-') into a Tk entry (or anything except a number) with a -textvariable bound to a C int or C double variable raises an error in Tcl/Tk 8.5.19, which was not the case with 8.5.18.

Example of such error:

##############################################
  can't set "fthresh": variable must have real value
  variable must have real value
      (write trace on "fthresh")
      invoked from within
  "$w insert insert $s"
      (procedure "tk::EntryInsert" line 12)
      invoked from within
  "tk::EntryInsert .display.le.fthresh.e -"
      (command bound to event)
##############################################

The OP C code looks like this:

##############################################
double fthresh;
static Tcl_Interp *interp;

int main(int argc, char **argv)
{
    ...
    interp = Tcl_CreateInterp();
    ...
    Tcl_LinkVar(interp,"fthresh",(char *)&fthresh, TCL_LINK_DOUBLE);
    ...
}
##############################################

And his Tcl code looks like this:

##############################################
   entry .a -textvariable fthresh ...
##############################################

A complete bug reproduction package (C code, Tcl code and instructions to run it) was provided by the OP and is attached to the present ticket.

Investigation of the issue revealed that:

1. Tcl_LinkVar does its job through a variable trace.

2. Fixing Tk bug 1700065 made the issue apparent because since that fix, when a write trace is set on a textvariable of an entry, and there is an error in that write trace proc, the behavior changed. The error became reported instead of being silently ignored.

3. The ttk::entry has and always had (AFAIK) this issue, even before fixing Tk bug 1700065 (which did not touch ttk::entry anyway).

4. Problem is not limited to inserting a bare minus (-) sign, it happens when inserting a bare plus (+) or dot (.) sign as well. All being potentially realistic actions you could want to do in an entry widget.

5. Looking in the Tcl source code, LinkTraceProc() simply checks if Tcl_GetIntFromObj() returns TCL_OK or not. And similarly for other types (wide int, double, short...). A bare -, +, or . makes this call return TCL_ERROR.

In conclusion, I believe that LinkTraceProc() could be more tolerant to what could be an integer or double.

Donald Arseneau proposed the following change:

If string converts to proper numeric, then apply value

else if string with "0" appended converts to proper numeric,

don't apply value, but don't give error either.

else raise error

@avl42: Yes, good catch! If we accept all prefixes of possible value integers, those 3-character prefixes should be accepted too. In practice, would anyone really type "+0xff" or "-0x05" in an entry widget?

I read over the changes in GetInvalidIntFromObj(), and it struck me: what about 3-chars prefixes of kind "[+-]0[xXbBoO]"? Wouldn't these still throw error?

I just compiled the latest 8.5's downloaded yesterday (24 Jan 2017):

   Tcl+Source+Code-2579d7ff89.tar.gz
   Tk+Source+Code-bfb8e49e.tar.gz

and tested them with tiny.c.  You can now have a blank entry or replace a valid int or double numerical value by typing:

   {minussign}{number}{etc}

in:

   tk::entry
   ttk::entry

without error.  Looks good!  Typing letters (except hex prefix), gives an error as it should.

cheers,
marty

Having a look.... stay tuned

Hmmm... at least I don't see any improvement with the latest commit on this. I see exactly what Marty reported regarding empty entries.

Jan I strongly suggest you test with Marty's "tiny" program, did you already?

Thanks, Marty! Tcl_LinkVar should now be tolerant to the empty string as well (in all branches).

For trunk, I would like to experiment with a slightly better (but more *potentially incompatible*) change. But I'll start with that as soon as *this* approach works. I'm not sure what's wrong with ttk:entry (maybe there's another bug in Tk related to that), but at least this should be another step in the right direction.

Marty Sereno (the OP) reports by email:

###########################################################
### tcl 8.5.19
tar xvfz Tcl+Source+Code-3bb0eb915b.tar.gz
cd Tcl_Source_Code-3bb0eb915b/unix
./configure
make
cd ../..
setenv TCL_LIBRARY ./Tcl_Source_Code-3bb0eb915b/library
setenv DYLD_LIBRARY ./Tcl_Source_Code-3bb0eb915b/unix
./Tcl_Source_Code-3bb0eb915b/unix/tclsh => OK

### tk 8.5.19
tar xvfz Tk+Source+Code-e11410b5.tar.gz
cd Tk_Source_Code-e11410b5/unix
./configure --with-tcl=../../Tcl_Source_Code-3bb0eb915b/unix
make
setenv TK_LIBRARY ./Tk_Source_Code-e11410b5/library
setenv DYLD_LIBRARY_PATH ./Tcl_Source_Code-3bb0eb915b/unix:./Tk_Source_Code-e11410b5/unix
./Tk_Source_Code-e11410b5/unix/wish => OK

### tiny.c, tiny.tcl
setenv LD_LIBRARY_PATH ./Tcl_Source_Code-3bb0eb915b/unix:./Tk_Source_Code-e11410b5/unix
set path = (./Tcl_Source_Code-3bb0eb915b/unix ./Tk_Source_Code-e11410b5/unix $path)
cp ../tiny.c .
gcc tiny.c -I/opt/X11/include -I./Tcl_Source_Code-3bb0eb915b/generic -I./Tk_Source_Code-e11410b5/generic -lX11 -L/opt/X11/lib -L./Tcl_Source_Code-3bb0eb915b/unix -L./Tk_Source_Code-e11410b5/unix -ltcl8.5 -ltk8.5 -o tiny
tiny
###########################################################

You can now type in a bare '-' (and '+', etc)
into an empty entry.

But one major difference from the previous state
is that now, there is a "variable must have integer/real
value" Application Error when you completely clear the
entry.

Say there is a "2" in the entry.

With tk::entry, you get an error if
you backspace to nothing, but no error
if you select what is in there and
begin typing "-2" (implied delete).

With ttk::entry, you get an error if
you backspace to nothing, but you
*also* get an error if you select what
is in there and begin typing "-2"
(implied delete).

Before, both tk::entry and ttk::entry
would allow the entry to be empty
without an error.

I think that an empty entry should still
be allowed.

I'm assigning to you, Jan, since you did the fix. Could you please have a further look? Thanks!

I have now prompted the OP twice by direct email about checking that the proposed fix works for him. Without answer I'll wait a few more days and then close this ticket.

Jan Nijtmans has seemingly fixed the problem in object, in branches:

   core-8-5-branch
   core-8-6-branch
   trunk

He merged the work he did in branch bug-39f6304c2e:
http://core.tcl.tk/tcl/timeline?r=bug-39f6304c2e&nd&c=2016-12-23+12%3A02%3A08&n=200
into the above branches.

Marty, could you please check these code changes do answer your 
problem, and report directly in the ticket:
   http://core.tcl.tk/tcl/info/39f6304c2e

The fix proposal looks good to me, does not break any new test for me (neither on Linux Debian 8 nor on Windows Vista), and there are new tests exercising the fix.

IMO the only remaining thing is that I have tried the OP's "./tiny" demo and have found the following:

  - It is still not possible to input "1e5" for instance in neither the entry nor the ttk::entry, in both the int- and double-linked cases. This was not dealt with by the fix. I believe it should work for the double-linked case.

  - Starting from a state where the field displays a valid value, say "10", select everything and type a bare minus sign. This work OK with an entry, i.e. the "-" gets displayed, but for a ttk::entry one still gets the same error as reported.

Try the OP's test case / bug reproduction package, it's nice for testing.

Rebased the proposal to core-8-5-branch ([a27bf1339327de90]), and removed the proposed changes to the Boolean/Integer/Double string representations. This change can harmlessly be done without a TIP in all active branches (core-8-5-branch and up).

François, Please review. It allows strings like "0x" and "+" be used in Tcl variables which are linked with integer C variables. So they can be typed in without causing trace errors. But in other contexts than that, "0x" and "+" are still NOT considered valid.

Thanks! Jan Nijtmans

Jan Nijtmans started crafting of a possible solution, see branch bug-39f6304c2e.

I have tried patches along the proposed lines, e.g. (just for int here):

Index: generic/tclLink.c
==================================================================
--- generic/tclLink.c
+++ generic/tclLink.c
@@ -380,14 +380,22 @@
 
     switch (linkPtr->type) {
     case TCL_LINK_INT:
        if (Tcl_GetIntFromObj(NULL, valueObj, &linkPtr->lastValue.i)
                != TCL_OK) {
-           Tcl_ObjSetVar2(interp, linkPtr->varName, NULL, ObjValue(linkPtr),
-                   TCL_GLOBAL_ONLY);
-           return "variable must have integer value";
-       }
+            Tcl_AppendObjToObj(valueObj, Tcl_NewStringObj("0", -1));
+            if (Tcl_GetIntFromObj(NULL, valueObj, &linkPtr->lastValue.i)
+                    != TCL_OK) {
+                Tcl_ObjSetVar2(interp, linkPtr->varName, NULL, ObjValue(linkPtr),
+                        TCL_GLOBAL_ONLY);
+                return "variable must have integer value";
+            } else {
+                Tcl_ObjSetVar2(interp, linkPtr->varName, NULL, ObjValue(linkPtr),
+                        TCL_GLOBAL_ONLY);
+                break;
+            }
+        }
        LinkedVar(int) = linkPtr->lastValue.i;
        break;
 
     case TCL_LINK_WIDE_INT:
        if (Tcl_GetWideIntFromObj(NULL, valueObj, &linkPtr->lastValue.w)

This results in now being able to erase the entry widget content without error (but without visually seeing the widget empty: the last value is still present) and to be able to enter - or +. But I don't find this very ergonomic and rather misleading for the user.

As a variant, setting the value to Tcl_NewObj() instead of ObjValue(linkPtr) has similar drawbacks as well.