[GRASS5] minor update to tcltkgrass for GRASS 5.7

I ran into a couple errors in tcltkgrass and corrected them. The corrected file has been committed to the CVS and uploaded to my website <www.public.asu.edu/~cmbarton/grass.htm>.

The errors prevented the following programs from starting:
d.barscale
d.scale
v.in.region

I also made d.out.png modal

Michael
______________________________
Michael Barton, Professor & Curator
School of Human Origins, Cultures, & Societies
Arizona State University
Tempe, AZ 85287-2402
USA

voice: 480-965-6262; fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

I ran into a couple errors in tcltkgrass and corrected them.

The errors prevented the following programs from starting:
d.barscale
d.scale
I also made d.out.png modal

d.scale is superseded by 'd.barscale -l' and doesn't exist in GRASS 5.7.
I've updated menu.tcl in cvs.

Additionally, d.barscale's GUI isn't rendering properly. When you open
it, the bcolor= and at= descriptors are missing, and the following error
is displayed on the console:

extra characters after close-quote
bad window path name ".pw.f0.frame.optwin.fra.frame.lab1"
invalid command name "0,0"
bad window path name ".pw.f0.frame.optwin.fra.frame.lab3"

It is choking on the " character in bcolor=; if I change the module to
use ' instead, it works.. this will break d.text.freetype as well.

Similarly, at= breaks when it sees , that happens in d.text as well.

It would be nice if the parser be a bit more robust to handle ".
Are there other chars which will break it as well? {}?

[d.barscale] Parameters:

  bcolor Background color, either a standard GRASS color, R:G:B
             triplet, or "none"
           default: white

  tcolor Text color, either a standard GRASS color or R:G:B triplet
             (separated by colons)
           default: black

      at The screen coordinates for top-left corner of label ([0,0] is
             top-left of frame)
           options: 0-100
           default: 0.0,0.0

Once again, I'd like some feedback on if [0%,0%] should be standardized
as top left or bottom left of the frame. As it is, we use both.
See 'd.text.freetype -p -n' for an example.. In that case, I vote for
removing -p altogether.

I vote for bottom-left when we are talking about percentage of frame,
which is what I think we should be using for screen coordinates most of
the time, except for labels. 5.7's d.vect deals with labels pretty well
though.
?

btw, d.text from the GUI doesn't work right now as it gets its input
from stdin. I've been meaning to add a text="" option like
d.text.freetype has, but there's still the gui quoting problem for
multiple words. I've also been meaning to update d.text and
d.text.freetype to use the mouse positioning code from d.barscale which
works nicely, but haven't gotten to it yet.

Hamish

> d.barscale's GUI isn't rendering properly. When you open
> it, the bcolor= and at= descriptors are missing, and the following
> error is displayed on the console:
>
> extra characters after close-quote
> bad window path name ".pw.f0.frame.optwin.fra.frame.lab1"
> invalid command name "0,0"
> bad window path name ".pw.f0.frame.optwin.fra.frame.lab3"
>
> It is choking on the " character in bcolor=; if I change the module
> to use ' instead, it works.. this will break d.text.freetype as
> well.
>
> Similarly, at= breaks when it sees , that happens in d.text too.
>
> It would be nice if the parser be a bit more robust to handle ".

Ok, I think I fixed this one.

solution: in grass51/lib/gis/parser.c, replace

... -text \"$description\" ...
with
... -text {$description} ...

this text is in a sprintf() statement & the string goes into a command
which is eventually fed through:

echo "$command" | wish &

for the archives:
To debug the tcl code, I had to add a line to parser.c just before
system(cmd);

sprintf(stdout, "%s\n", cmd);
  [perhaps this should be a if(DEBUG=3) command?]

then run

GRASS:~> d.text.freetype > dtf_tcl.txt

then edit 'dtf_tcl.txt' to remove the "| wish &" from the end.

then '. dtf_tcl.txt > dtf.tcl' to get rid of the echo-proofing chars.

then 'wish < dtf.tcl' to test it.

It was easiest to fiddle with dtf.tcl until it was right and then go
back and fix parser.c to provide what was needed.

References:
"Is white space significant in Tcl" http://wiki.tcl.tk/981
"Tcl quoting hell" http://wiki.tcl.tk/1726
"1 minute intro to tcl" http://www.pconline.com/~erc/tcl.htm

Next big bug is to get the Tcl menus to honour white spaces:

d.vect where="ident='foo' and location='bar'"

prints out to the stdout window with the double quotes when you click "Run",
but they get stripped before the command is run causing it to break.

Hamish

for the archives:
To debug the tcl code, I had to add a line to parser.c just before
system(cmd);

sprintf(stdout, "%s\n", cmd);
  [perhaps this should be a if(DEBUG=3) command?]

ummm; yeah... it's already in there.
g.gisenv set="DEBUG=1"

then run

GRASS:~> d.text.freetype > dtf_tcl.txt

then edit 'dtf_tcl.txt' to remove the "| wish &" from the end.

..

then 'wish < dtf_tcl.txt' to test it.

It was easiest to fiddle with dtf.tcl until it was right and then go
back and fix parser.c to provide what was needed.

References:
"Is white space significant in Tcl" http://wiki.tcl.tk/981
"Tcl quoting hell" http://wiki.tcl.tk/1726
"1 minute intro to tcl" http://www.pconline.com/~erc/tcl.htm

Next big bug is to get the Tcl menus to honour white spaces:

d.vect where="ident='foo' and location='bar'"

prints out to the stdout window with the double quotes when you click
"Run", but they get stripped before the command is run causing it to
break.

I'd guess the problem would be in here somehwere?

button .run -text "Run" -command {
    global outtext pipe
    set cmd [ mkcmd ]
    $outtext insert end "\n$cmd\n"
    $outtext yview end
    set cmd "| $cmd 2>@ stdout"
    catch {open $cmd r} msg
    fconfigure $msg -blocking 0
    fileevent $msg readable [ list prnout $msg ]
    update idletasks
}

?,
Hamish

Hamish wrote:

this text is in a sprintf() statement & the string goes into a command
which is eventually fed through:

echo "$command" | wish &

That's the real bug; the rest are just symptoms.

The way that G_gui() currently works is absolutely crazy. For two main
reasons:

1. The actual text in parser.c is first being interpreted according to
the syntax of C string literals, then the result is being interpreted
according to the syntax of /bin/sh (due to the use of system()), then
that gets interpreted according to the syntax of "echo" (at least
that's only an issue for backslashes), then the ultimate result is
finally interpreted as Tcl code.

2. It ends up calling system() with a potentially massive argument (it
allocates 100Kb). Many Unices have a limit to the length of the
command line, and it's typically less than 100Kb; 4Kb is typical.

If you're planning on doing more work on this, you could probably save
yourself (and anyone else who works on it) some work by:

1. Eliminating the use of the shell and "echo", by using popen()
instead of system(). Then, whatever you write to the FILE* is sent
directly to wish's stdin, without any processing. Apart from avoiding
the processing, it also means that you can send it a bit at a time
with fprintf/fputs; you don't need to buffer the whole thing in
memory.

2. Moving all of the boilerplate into a prologue which defines
procedures. Then, G_gui() just needs to send e.g.
"source $env(GISBASE)/etc/prolog.tcl" at the start, and you only need
to generate the bits which will actually change between calls. Also,
the bulk text which is moved into the prologue wouldn't need to be
encoded as C string literals.

A couple of other points about that code:

1. In Tcl, everything is a string. You don't need to quote strings
unless they contain spaces, so e.g.:

  append(cmd, "set opttype(%d) \"multi\" \n", optn);

can be written as just:

  append(cmd, "set opttype(%d) multi \n", optn);

2. This sort of thing is asking for trouble:

  append(cmd, " \"%s\" ", s );

If the value of s contains any Tcl metacharacters, the end result
probably isn't going to be what was desired. [With the current
implementation, the same applies to shell or echo metacharacters.]

It would be (slightly) more reliable to use braces, i.e.

  append(cmd, " {%s} ", s );

However, that fails if the value of s itself contains braces, and
there isn't any way to include a literal left or right brace within
braces (backslash-brace will prevent the brace from being interpreted
for the purposes of nesting, but the backslash will be retained).

The only reliable solution is to use double quotes and to precede any
metacharacter (double quote, left bracket, dollar or backslash) with a
backslash. Which means you can't just use fprintf("... %s ..."); you
need a function which will scan the string and insert backslashes as
required.

References:

"Tcl quoting hell" http://wiki.tcl.tk/1726

Hehe.

Although you should really provide "bash quoting hell" and "echo
quoting hell" references if system("echo ...") is going to be used.

Actually, this kind of problem is inherent with any form of "in-band
signalling" mechanism, of which string literals (in most languages),
printf(), /bin/sh and Tcl are all examples. It's particularly
problematic when it forms the basis of an entire language (as is the
case for most "scripting" languages).

And it's bound to be problematic when you combine four different
in-band signalling mechanisms together (i.e. C string literals,
vsprintf, /bin/sh and Tcl).

[Aside: if you read BugTraq long enough, you will eventually realise
that 95% of security vulnerabilities occur for one of two reasons. One
is buffer overflows. The other is in-band signalling, i.e. where you
end up inserting user-supplied signalling codes when you thought that
you were just inserting data.]

--
Glynn Clements <glynn.clements@virgin.net>

On Sun, Aug 01, 2004 at 06:53:16PM +1200, Hamish wrote:

> > d.barscale's GUI isn't rendering properly. When you open
> > it, the bcolor= and at= descriptors are missing, and the following
> > error is displayed on the console:
> >
> > extra characters after close-quote
> > bad window path name ".pw.f0.frame.optwin.fra.frame.lab1"
> > invalid command name "0,0"
> > bad window path name ".pw.f0.frame.optwin.fra.frame.lab3"
> >
> > It is choking on the " character in bcolor=; if I change the module
> > to use ' instead, it works.. this will break d.text.freetype as
> > well.
> >
> > Similarly, at= breaks when it sees , that happens in d.text too.
> >
> > It would be nice if the parser be a bit more robust to handle ".

Ok, I think I fixed this one.

solution: in grass51/lib/gis/parser.c, replace

... -text \"$description\" ...
with
... -text {$description} ...

Thanks a lot! This was a long-standing problem...

Markus

Glynn Clements wrote:

If you're planning on doing more work on this, you could probably save
yourself (and anyone else who works on it) some work by:

1. Eliminating the use of the shell and "echo", by using popen()
instead of system(). Then, whatever you write to the FILE* is sent
directly to wish's stdin, without any processing. Apart from avoiding
the processing, it also means that you can send it a bit at a time
with fprintf/fputs; you don't need to buffer the whole thing in
memory.

2. Moving all of the boilerplate into a prologue which defines
procedures. Then, G_gui() just needs to send e.g.
"source $env(GISBASE)/etc/prolog.tcl" at the start, and you only need
to generate the bits which will actually change between calls. Also,
the bulk text which is moved into the prologue wouldn't need to be
encoded as C string literals.

FWIW, I've just done this. Well, I've only partially done #2; the
totally-boilerplate stuff has been moved into the prologue
(lib/gis/gui.tcl), but some of the mostly-boilerplate stuff is still
in G_gui().

--
Glynn Clements <glynn.clements@virgin.net>