[GRASS-dev] d.rast.edit in wxgrass

d.rast.edit ought to run from wxgrass, assuming that TclTk is installed. It launches the wxPython properties dialog when called from the menu. However, the wxPython properties dialog, then launches the TclTk properties dialog instead of just running the command (note that running d.rast.edits with arguments runs fine from the wxgrass built in CLI).

The place where the command is getting rerouted to call the TclTk dialog seems to be in line 636 of menuform.py

retcode = subprocess.call(cmd, shell=True)

Any idea why this might be? I could hack around this for the one command, but there should be a more generic solution it seems.

BTW, I got the xterm launching method to work by using subprocess.Popen(cmdlist) for starting the needed xmon [‘d.mon’, ‘start=xmon’]. Both cmd.Command(cmdlist) and cmd.Command(cmdlist, wait=False) didn’t work and hung up waiting on the xmon. Don’t know why.

Michael


Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

Michael Barton wrote:

d.rast.edit ought to run from wxgrass, assuming that TclTk is installed. It
launches the wxPython properties dialog when called from the menu. However,
the wxPython properties dialog, then launches the TclTk properties dialog
instead of just running the command (note that running d.rast.edits with
arguments runs fine from the wxgrass built in CLI).

The place where the command is getting rerouted to call the TclTk dialog
seems to be in line 636 of menuform.py

It's being treated as a layer because it begins with "d.".

The "d.*" hack is a bad idea; I suggest that you remove it.

retcode = subprocess.call(cmd, shell=True)

Why are you using a string instead of a list?

Any idea why this might be? I could hack around this for the one command,
but there should be a more generic solution it seems.

BTW, I got the xterm launching method to work by using
subprocess.Popen(cmdlist) for starting the needed xmon ['d.mon',
'start=xmon']. Both cmd.Command(cmdlist) and cmd.Command(cmdlist,
wait=False) didn't work and hung up waiting on the xmon. Don't know why.

Personally, I suggest not bothering with interactive d.* commands.
wxgrass isn't going to be stable until 7.x (apart from anything else,
wxPython 2.8 isn't going to be in widespread use before then), so I
don't see much point in making an effort to support features which
will have been removed by then.

--
Glynn Clements <glynn@gclements.plus.com>

Glynn,

On 6/12/07 8:58 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

Michael Barton wrote:

d.rast.edit ought to run from wxgrass, assuming that TclTk is installed. It
launches the wxPython properties dialog when called from the menu. However,
the wxPython properties dialog, then launches the TclTk properties dialog
instead of just running the command (note that running d.rast.edits with
arguments runs fine from the wxgrass built in CLI).

The place where the command is getting rerouted to call the TclTk dialog
seems to be in line 636 of menuform.py

It's being treated as a layer because it begins with "d.".

The "d.*" hack is a bad idea; I suggest that you remove it.

I'm not sure why the d.* clause is in this particular place--one reason to
query Daniel who has done most of the work making menuform work well.

We need to split out d.* commands elsewhere because they must be processed
differently from other commands in order to show up in the display canvas,
but this would not apply to d.rast.edit. It is the only command that I know
of that would handle it's own display still.

retcode = subprocess.call(cmd, shell=True)

Why are you using a string instead of a list?

I looked at cmd coming in and I think it is a list.

Any idea why this might be? I could hack around this for the one command,
but there should be a more generic solution it seems.

BTW, I got the xterm launching method to work by using
subprocess.Popen(cmdlist) for starting the needed xmon ['d.mon',
'start=xmon']. Both cmd.Command(cmdlist) and cmd.Command(cmdlist,
wait=False) didn't work and hung up waiting on the xmon. Don't know why.

Personally, I suggest not bothering with interactive d.* commands.
wxgrass isn't going to be stable until 7.x (apart from anything else,
wxPython 2.8 isn't going to be in widespread use before then), so I
don't see much point in making an effort to support features which
will have been removed by then.

Well, my idea was to simply make it possible to call the few remaining
commands like nviz d.rast.edit, that still need TlcTk canvases, and ones
that still need an xterm, rather than really supporting them (i.e., no
further development on their interfaces the way they are now). That way
their functionality remains available while we work to replace them with
wxPython. Kind of smoothing the transition somewhat.

It should be possible to port d.rast.edit (or its functionality) to
wxPython, since it is TclTk, at which time we'd just change the call in the
menu to aim at the new one. Nviz launches fine, as do the xterm commands.
d.rast.edit is the only one that doesn't launch quite right. It's not a
biggie, but if it is an easy fix, it would be nice.

Michael

__________________________________________
Michael Barton, Professor of Anthropology and Director of Graduate Studies
School of Human Evolution & Social Change
Center for Social Dynamics and Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

Michael Barton wrote:

>> d.rast.edit ought to run from wxgrass, assuming that TclTk is installed. It
>> launches the wxPython properties dialog when called from the menu. However,
>> the wxPython properties dialog, then launches the TclTk properties dialog
>> instead of just running the command (note that running d.rast.edits with
>> arguments runs fine from the wxgrass built in CLI).
>>
>> The place where the command is getting rerouted to call the TclTk dialog
>> seems to be in line 636 of menuform.py
>
> It's being treated as a layer because it begins with "d.".
>
> The "d.*" hack is a bad idea; I suggest that you remove it.

I'm not sure why the d.* clause is in this particular place--one reason to
query Daniel who has done most of the work making menuform work well.

We need to split out d.* commands elsewhere because they must be processed
differently from other commands in order to show up in the display canvas,
but this would not apply to d.rast.edit. It is the only command that I know
of that would handle it's own display still.

d.* commands used as map layers belong in a separate menu. The menu
items for layer commands should have different bindings to normal
commands.

Determining how to run a command based upon its name is simply wrong.

>> retcode = subprocess.call(cmd, shell=True)
>
> Why are you using a string instead of a list?

I looked at cmd coming in and I think it is a list.

In which case, the shell=True doesn't belong there.

It should be possible to port d.rast.edit (or its functionality) to
wxPython, since it is TclTk, at which time we'd just change the call in the
menu to aim at the new one. Nviz launches fine, as do the xterm commands.
d.rast.edit is the only one that doesn't launch quite right. It's not a
biggie, but if it is an easy fix, it would be nice.

The fix for d.rast.edit is to remove the hack which treats d.*
commands differently.

--
Glynn Clements <glynn@gclements.plus.com>

On 6/12/07 9:25 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

Michael Barton wrote:

d.rast.edit ought to run from wxgrass, assuming that TclTk is installed. It
launches the wxPython properties dialog when called from the menu. However,
the wxPython properties dialog, then launches the TclTk properties dialog
instead of just running the command (note that running d.rast.edits with
arguments runs fine from the wxgrass built in CLI).

The place where the command is getting rerouted to call the TclTk dialog
seems to be in line 636 of menuform.py

It's being treated as a layer because it begins with "d.".

The "d.*" hack is a bad idea; I suggest that you remove it.

I'm not sure why the d.* clause is in this particular place--one reason to
query Daniel who has done most of the work making menuform work well.

We need to split out d.* commands elsewhere because they must be processed
differently from other commands in order to show up in the display canvas,
but this would not apply to d.rast.edit. It is the only command that I know
of that would handle it's own display still.

d.* commands used as map layers belong in a separate menu. The menu
items for layer commands should have different bindings to normal
commands.

Determining how to run a command based upon its name is simply wrong.

This is needed, at least in some spots to permit running d.* commands from
the CLI. That's why it needs to look at the name.

If it was only from the menu, then it would be easy. I don't know if this is
the case in menuform. I don't know exactly what is going on in this spot and
haven't had time yet to figure it out.

retcode = subprocess.call(cmd, shell=True)

Why are you using a string instead of a list?

I looked at cmd coming in and I think it is a list.

In which case, the shell=True doesn't belong there.

This may be the immediate problem. I'll test.

Michael

It should be possible to port d.rast.edit (or its functionality) to
wxPython, since it is TclTk, at which time we'd just change the call in the
menu to aim at the new one. Nviz launches fine, as do the xterm commands.
d.rast.edit is the only one that doesn't launch quite right. It's not a
biggie, but if it is an easy fix, it would be nice.

The fix for d.rast.edit is to remove the hack which treats d.*
commands differently.

__________________________________________
Michael Barton, Professor of Anthropology and Director of Graduate Studies
School of Human Evolution & Social Change
Center for Social Dynamics and Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

Michael Barton wrote:

> d.* commands used as map layers belong in a separate menu. The menu
> items for layer commands should have different bindings to normal
> commands.
>
> Determining how to run a command based upon its name is simply wrong.

This is needed, at least in some spots to permit running d.* commands from
the CLI. That's why it needs to look at the name.

No, it's needed to force d.* commands to be intercepted and used to
create layers. It won't work if the d.* command isn't suitable as a
layer command (e.g. d.rast.edit). It also precludes running d.*
commands normally (i.e. using a monitor).

Ignoring the last point, determining whether or not a command can be
used as a layer command isn't as simple as matching against "d.*".

If it was only from the menu, then it would be easy. I don't know if this is
the case in menuform. I don't know exactly what is going on in this spot and
haven't had time yet to figure it out.

The name "menuform.py" suggests that it has something to do with
menus. In which case, what is:

        if cmd[0][0:2] != "d.":

doing in that file (specifically, in mainFrame.OnRun())?

Even if you are determined to use that hack for the CLI, it has no
utility in regard to menus.

--
Glynn Clements <glynn@gclements.plus.com>

On 6/13/07 8:07 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

Michael Barton wrote:

d.* commands used as map layers belong in a separate menu. The menu
items for layer commands should have different bindings to normal
commands.

Determining how to run a command based upon its name is simply wrong.

This is needed, at least in some spots to permit running d.* commands from
the CLI. That's why it needs to look at the name.

No, it's needed to force d.* commands to be intercepted and used to
create layers. It won't work if the d.* command isn't suitable as a
layer command (e.g. d.rast.edit). It also precludes running d.*
commands normally (i.e. using a monitor).

Ignoring the last point, determining whether or not a command can be
used as a layer command isn't as simple as matching against "d.*".

Clarification.

No d.* commands come from the menus, except for d.rast.edit and some old
ones that need xmons. The latter are specially routed through the OnXterm
method and never are parsed by menuform.py. d.rast.edit is the only weird
exception solely because you made a nice gui for it.

"Normal" menu commands are all those that are not display commands and have
no arguments. Explicitly in the menu, these are processed by OnMenuCmd in
wxgui.py. They are sent to the menuform.py parser. d.* commands (e.g.,
d.rast, d.grid, etc) are simply not in the menu as a general rule because
they need to handled differently, as you say.

Menu commands with arguments (e.g., g.region -p) are explicitly handled by
RunMenuCmd, which simply sends them on to the command processor (ultimately
cmd.Command(cmdlist), rather than menuform.py.

Menu commands which call wxPython modules have their own handlers (e.g.,
histogramming or the new rules dialogs).

Menu commands which require an xterm (a few like r.digit) are handled by the
new OnXTerm method, which launches an xmon and xterm.

Jachym and others have been working on some Python scripts (e.g., p.mon) to
give access to some the of the wxgrass functions from the *nix terminal. I
don't really know how these function. The d.* trap within menuform.py may be
related to these or it may be a holdover from an earlier iteration that
should be dropped.

For the CLI built into wxgrass (the one I DO know about), here is the
parsing logic.

A list of GRASS CLI commands is made by searching $GISBASE/bin and
$GISBASE/scripts.

If the first part (i.e., before the first space) of a string typed on the
CLI is not in the list, it is passed on to the shell.

If first part of the string IS in the GRASS command list, it is treated as a
GRASS command. I realize that the following is not ideal, but it's the best
I've been able to come up with so far and covers the great majority of
cases. Suggestions for improvements are welcome.

GRASS command parsing from the wxgrass CLI:

Split the command into a list by spaces. I realize that this is a problem
for any command with spaces in the arguments, but I know of no better way to
do this in this context outside of making users type any command as a Python
list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
g.region rast=mymap res=30, I don't think anyone would want to use the CLI.

If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
d.* command, send the original command string (not the list) to the
menuform.py processor (just like the menu would) to create the autogenerated
properties dialog. After this, the result of hitting "run" is the same as if
the properties dialog were called from the menu.

If the command has no arguments AND IS a d.* command AND is in a hard coded
list of d.* commands for which we have defined layers in the layer tree,
create a layer tree item for that command (e.g., a raster layer for d.rast).
After that, the new layer is processed in the same way as one created by
pressing a layer button on the layer manager toolbar.

If the d.* command without arguments is NOT on the list for the layer tree,
generate get a message that it cannot be processed.

If the command HAS arguments and is NOT a d.* command, send the command list
(i.e., made by splitting on spaces -- not the original command string) to
cmd.Command for processing.

If the command HAS arguments and IS a d.* command, it is added to the list
of display layers managed within render.py for the map display that has the
focus and the UpdateMap method is called to update that display. There is a
2nd splitting that allows for multiple d.* commands to be separated by
semi-colons. This may or may not be a good idea in the end, but I thought
I'd see how it worked out.

If it was only from the menu, then it would be easy. I don't know if this is
the case in menuform. I don't know exactly what is going on in this spot and
haven't had time yet to figure it out.

The name "menuform.py" suggests that it has something to do with
menus. In which case, what is:

        if cmd[0][0:2] != "d.":

doing in that file (specifically, in mainFrame.OnRun())?

I don't know.

menuform.py parses the --interface-description xml data to create a
properties dialog. The dialog takes the results of user choices and creates
a command list for processing--ulitmately by cmd.Command.

Even if you are determined to use that hack for the CLI, it has no
utility in regard to menus.

I just don't know if it is related to the p.mon, p.rast, etc scripts or
whether it can simply be removed.

Michael

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

Michael Barton wrote:

GRASS command parsing from the wxgrass CLI:

Split the command into a list by spaces. I realize that this is a problem
for any command with spaces in the arguments, but I know of no better way to
do this in this context outside of making users type any command as a Python
list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
g.region rast=mymap res=30, I don't think anyone would want to use the CLI.

This is one case where shell=True *is* legitimate. At least with a
shell, the user can use quotes or backslashes to include spaces in
arguments. Or you could implement equivalent functionality yourself.

The latter is more work, but you can choose exactly which shell
functionality you want, e.g. allowing spaces (and other
metacharacters) without the other shell behaviour (sequences,
pipelines, redirection, variables, ....).

If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
d.* command, send the original command string (not the list) to the
menuform.py processor (just like the menu would) to create the autogenerated
properties dialog. After this, the result of hitting "run" is the same as if
the properties dialog were called from the menu.

If the command has no arguments AND IS a d.* command AND is in a hard coded
list of d.* commands for which we have defined layers in the layer tree,
create a layer tree item for that command (e.g., a raster layer for d.rast).
After that, the new layer is processed in the same way as one created by
pressing a layer button on the layer manager toolbar.

If the d.* command without arguments is NOT on the list for the layer tree,
generate get a message that it cannot be processed.

If the command HAS arguments and is NOT a d.* command, send the command list
(i.e., made by splitting on spaces -- not the original command string) to
cmd.Command for processing.

If the command HAS arguments and IS a d.* command, it is added to the list
of display layers managed within render.py for the map display that has the
focus and the UpdateMap method is called to update that display. There is a
2nd splitting that allows for multiple d.* commands to be separated by
semi-colons. This may or may not be a good idea in the end, but I thought
I'd see how it worked out.

This all looks suspiciously like DWIM, which is normally a bad thing
for non-trivial systems.

>> If it was only from the menu, then it would be easy. I don't know if this is
>> the case in menuform. I don't know exactly what is going on in this spot and
>> haven't had time yet to figure it out.
>
> The name "menuform.py" suggests that it has something to do with
> menus. In which case, what is:
>
> if cmd[0][0:2] != "d.":
>
> doing in that file (specifically, in mainFrame.OnRun())?

I don't know.

menuform.py parses the --interface-description xml data to create a
properties dialog. The dialog takes the results of user choices and creates
a command list for processing--ulitmately by cmd.Command.

> Even if you are determined to use that hack for the CLI, it has no
> utility in regard to menus.

I just don't know if it is related to the p.mon, p.rast, etc scripts or
whether it can simply be removed.

As a general rule, if no-one knows why something is there, then it
shouldn't be there.

It's understandable if something as old as GRASS has legacy code for
which the reason is unknown, but wxgrass shouldn't have reached that
point already.

--
Glynn Clements <glynn@gclements.plus.com>

On 6/14/07 12:28 PM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

The name "menuform.py" suggests that it has something to do with
menus. In which case, what is:

        if cmd[0][0:2] != "d.":

doing in that file (specifically, in mainFrame.OnRun())?

I don't know.

menuform.py parses the --interface-description xml data to create a
properties dialog. The dialog takes the results of user choices and creates
a command list for processing--ulitmately by cmd.Command.

Even if you are determined to use that hack for the CLI, it has no
utility in regard to menus.

I just don't know if it is related to the p.mon, p.rast, etc scripts or
whether it can simply be removed.

As a general rule, if no-one knows why something is there, then it
shouldn't be there.

Daniel Cavelo knows. I don't.

It's understandable if something as old as GRASS has legacy code for
which the reason is unknown, but wxgrass shouldn't have reached that
point already.

Hopefully we're not creating legacy code already.

It is just that, for the first time, there is an actual team working on the
GUI. It is really nice, even if it means that I don't know the details of
certain parts of the code.

While I've done some work on the module called menuform.py, Daniel has done
the greatest amount and would know what is going on here. He seems out of
touch at the moment (as I will be next week).

Michael

__________________________________________
Michael Barton, Professor of Anthropology and Director of Graduate Studies
School of Human Evolution & Social Change
Center for Social Dynamics and Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

On 6/14/07 12:28 PM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

If the command HAS arguments and IS a d.* command, it is added to the list
of display layers managed within render.py for the map display that has the
focus and the UpdateMap method is called to update that display. There is a
2nd splitting that allows for multiple d.* commands to be separated by
semi-colons. This may or may not be a good idea in the end, but I thought
I'd see how it worked out.

This all looks suspiciously like DWIM, which is normally a bad thing
for non-trivial systems.

I don't think it is a DWIM. The normal CLI behavior currently is...

1) type a command without arguments and it (normally) opens a properties
dialog;

2) type a command with arguments and it runs the command. To run a d.*
command and have it do anything useful in the wxgrass environment, it needs
to be added to the list of display layers and the display updated. There is
no straightforward way to choose which of multiple open display windows
would show the d.* command results, so I'm simply picking the active one.
This can be changed by the user by simply clicking on a different display.

The added feature, which is particularly useful for running display commands
is the ability to 'chain' them together so you can create a display with
multiple overlaying maps. This is done here by separating d.* commands with
a semicolon. I'm not sure if this is the best way to achieve this end, but
it is convenient to have something along this line.

Michael

__________________________________________
Michael Barton, Professor of Anthropology and Director of Graduate Studies
School of Human Evolution & Social Change
Center for Social Dynamics and Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

On 6/14/07 12:28 PM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

GRASS command parsing from the wxgrass CLI:

Split the command into a list by spaces. I realize that this is a problem
for any command with spaces in the arguments, but I know of no better way to
do this in this context outside of making users type any command as a Python
list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
g.region rast=mymap res=30, I don't think anyone would want to use the CLI.

This is one case where shell=True *is* legitimate. At least with a
shell, the user can use quotes or backslashes to include spaces in
arguments. Or you could implement equivalent functionality yourself.

IIUC, if I keep these as strings, I don't think I can use the cmd.Command
class and would have to do a custom version of Popen. Then I'd have to send
them through a different processing path rather than the RunCmd method that
everything else uses.

Michael

The latter is more work, but you can choose exactly which shell
functionality you want, e.g. allowing spaces (and other
metacharacters) without the other shell behaviour (sequences,
pipelines, redirection, variables, ....).

__________________________________________
Michael Barton, Professor of Anthropology and Director of Graduate Studies
School of Human Evolution & Social Change
Center for Social Dynamics and Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

Michael Barton wrote:

>> GRASS command parsing from the wxgrass CLI:
>>
>> Split the command into a list by spaces. I realize that this is a problem
>> for any command with spaces in the arguments, but I know of no better way to
>> do this in this context outside of making users type any command as a Python
>> list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
>> g.region rast=mymap res=30, I don't think anyone would want to use the CLI.
>
> This is one case where shell=True *is* legitimate. At least with a
> shell, the user can use quotes or backslashes to include spaces in
> arguments. Or you could implement equivalent functionality yourself.

IIUC, if I keep these as strings, I don't think I can use the cmd.Command
class and would have to do a custom version of Popen. Then I'd have to send
them through a different processing path rather than the RunCmd method that
everything else uses.

One option is to call the shell explicitly, i.e.:

  cmd.Command(['/bin/sh', '-c', cmdstr], ....)

or (on Windows):

  cmd.Command(['cmd', '/c', cmdstr], ...)

Simply adding quoting to the existing implementation would probably be
preferable, though.

--
Glynn Clements <glynn@gclements.plus.com>

Michael Barton wrote:

>> If the command HAS arguments and IS a d.* command, it is added to the list
>> of display layers managed within render.py for the map display that has the
>> focus and the UpdateMap method is called to update that display. There is a
>> 2nd splitting that allows for multiple d.* commands to be separated by
>> semi-colons. This may or may not be a good idea in the end, but I thought
>> I'd see how it worked out.
>
> This all looks suspiciously like DWIM, which is normally a bad thing
> for non-trivial systems.
>

I don't think it is a DWIM. The normal CLI behavior currently is...

1) type a command without arguments and it (normally) opens a properties
dialog;

2) type a command with arguments and it runs the command. To run a d.*
command and have it do anything useful in the wxgrass environment, it needs
to be added to the list of display layers and the display updated.

Not all d.* commands generate graphics. Admittedly, most of the ones
which don't are only useful with a standalone monitor. I don't know
whether there are any other exceptions apart from d.rast.edit
(technically, the original v.digit should have had a d.* prefix, e.g.
d.vect.edit, but it doesn't).

For the d.* hack to work, we will need to adopt (and enforce) a rule
that the d.* prefix is reserved solely for commands which generate
graphics. That will become more practical in 7.x, as the display
"management" commands (d.mon, d.save, d.info etc) will no longer be
relevant once standalone monitors cease to exist.

--
Glynn Clements <glynn@gclements.plus.com>

Martin,

Is this change in cmd.Command easy to implement?

Michael

On 6/15/07 9:09 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

Michael Barton wrote:

GRASS command parsing from the wxgrass CLI:

Split the command into a list by spaces. I realize that this is a problem
for any command with spaces in the arguments, but I know of no better way
to
do this in this context outside of making users type any command as a
Python
list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead
of
g.region rast=mymap res=30, I don't think anyone would want to use the CLI.

This is one case where shell=True *is* legitimate. At least with a
shell, the user can use quotes or backslashes to include spaces in
arguments. Or you could implement equivalent functionality yourself.

IIUC, if I keep these as strings, I don't think I can use the cmd.Command
class and would have to do a custom version of Popen. Then I'd have to send
them through a different processing path rather than the RunCmd method that
everything else uses.

One option is to call the shell explicitly, i.e.:

cmd.Command(['/bin/sh', '-c', cmdstr], ....)

or (on Windows):

cmd.Command(['cmd', '/c', cmdstr], ...)

Simply adding quoting to the existing implementation would probably be
preferable, though.

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

On 6/15/07 9:21 AM, "Glynn Clements" <glynn@gclements.plus.com> wrote:

2) type a command with arguments and it runs the command. To run a d.*
command and have it do anything useful in the wxgrass environment, it needs
to be added to the list of display layers and the display updated.

Not all d.* commands generate graphics. Admittedly, most of the ones
which don't are only useful with a standalone monitor. I don't know
whether there are any other exceptions apart from d.rast.edit
(technically, the original v.digit should have had a d.* prefix, e.g.
d.vect.edit, but it doesn't).

For the d.* hack to work, we will need to adopt (and enforce) a rule
that the d.* prefix is reserved solely for commands which generate
graphics. That will become more practical in 7.x, as the display
"management" commands (d.mon, d.save, d.info etc) will no longer be
relevant once standalone monitors cease to exist.

Good point. The current hack is to have a list of acceptable d.* commands
that will actually work in the display. This needs to be extended to d.*
commands with arguments. If something is not on the list, it generates a
message dialog telling the user that this command is not implemented.

Michael

__________________________________________
Michael Barton, Professor of Anthropology
School of Human Evolution & Social Change
Center for Social Dynamics & Complexity
Arizona State University

phone: 480-965-6213
fax: 480-965-7671
www: http://www.public.asu.edu/~cmbarton

On Wed, 13 Jun 2007, Michael Barton wrote:

GRASS command parsing from the wxgrass CLI:

Split the command into a list by spaces. I realize that this is a problem
for any command with spaces in the arguments, but I know of no better way to
do this in this context outside of making users type any command as a Python
list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
g.region rast=mymap res=30, I don't think anyone would want to use the CLI.

If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
d.* command, send the original command string (not the list) to the
menuform.py processor (just like the menu would) to create the autogenerated
properties dialog. After this, the result of hitting "run" is the same as if
the properties dialog were called from the menu.

I don't get why it needs to do that specific processing - can you not just run whatever is entered - if a command is run without arguments it will pop up the GUI dialog anyway. Unless GRASS_UI_TERM is set. I suppose at the minute it will always be a Tcl/Tk dialog that pops up, but I'm sure it wouldn't be very hard to change G_parser() to check the GRASS_GUI variable (or do something similar) and generate the correct autogenerated dialog corresponding to the GUI that's in use.

If the command has no arguments AND IS a d.* command AND is in a hard coded
list of d.* commands for which we have defined layers in the layer tree,
create a layer tree item for that command (e.g., a raster layer for d.rast).
After that, the new layer is processed in the same way as one created by
pressing a layer button on the layer manager toolbar.

Sounds neat, consistent with non-display commands.

If the d.* command without arguments is NOT on the list for the layer tree,
generate get a message that it cannot be processed.

If the command HAS arguments and is NOT a d.* command, send the command list
(i.e., made by splitting on spaces -- not the original command string) to
cmd.Command for processing.

I think Glynn commented on this in a later mail but could you not just take the whole string the user entered and pass it to the shell (or cmd.exe on Windows) for executing - that way all the quoting rules etc. that work in the shell will be consistent with how it works here and reduce user confusion, and avoid you having to implement quoting handling yourself in the space-splitting algorithm..

If the command HAS arguments and IS a d.* command, it is added to the list
of display layers managed within render.py for the map display that has the
focus and the UpdateMap method is called to update that display. There is a

That's a nice idea. I didn't realise it was so easily possible. Is it
possible to edit a display layer by editing the underlying raw d.* command-line that it corresponds to, as an alternative to ticking boxes? There obviously must be code to convert the GUI representation of the layer to d.* command-line for drawing, and it sounds like you now have new code to parse the d.* command-line and convert it to a GUI representation (i.e. the display layer), so allowing editing of a layer by changing the raw command-line it uses should be possible.

2nd splitting that allows for multiple d.* commands to be separated by
semi-colons. This may or may not be a good idea in the end, but I thought
I'd see how it worked out.

Shell uses semi-colons. But you could you not just enter the two commands separately one after the other? I suppose if it was something that would take a long time to draw then that would be slow.

All sounds good. I like the idea of the GUI having command-line features and most of the command-line functionality being still available should you need it. You could call it power-user features I suppose.

Paul

Michael Barton wrote:

>>>> GRASS command parsing from the wxgrass CLI:
>>>>
>>>> Split the command into a list by spaces. I realize that this is a problem
>>>> for any command with spaces in the arguments, but I know of no better way
>>>> to
>>>> do this in this context outside of making users type any command as a
>>>> Python
>>>> list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead
>>>> of
>>>> g.region rast=mymap res=30, I don't think anyone would want to use the CLI.
>>>
>>> This is one case where shell=True *is* legitimate. At least with a
>>> shell, the user can use quotes or backslashes to include spaces in
>>> arguments. Or you could implement equivalent functionality yourself.
>>
>> IIUC, if I keep these as strings, I don't think I can use the cmd.Command
>> class and would have to do a custom version of Popen. Then I'd have to send
>> them through a different processing path rather than the RunCmd method that
>> everything else uses.
>
> One option is to call the shell explicitly, i.e.:
>
> cmd.Command(['/bin/sh', '-c', cmdstr], ....)
>
> or (on Windows):
>
> cmd.Command(['cmd', '/c', cmdstr], ...)
>
> Simply adding quoting to the existing implementation would probably be
> preferable, though.

Martin,

Is this change in cmd.Command easy to implement?

The above isn't a change to cmd.Command(), but a change to how it is
used.

If you were to change cmd.Command(), the obvious change would be to
add a shell= parameter, which would be propagated directly to Popen().

Ultimately, if all you have is a single string provided by the user,
that string somehow has to be converted to a list of strings (as
that's what argv is).

The options are:

1. Split at each space.
2. As for 1, but with a quoting and/or escaping mechanism.
3. Use the shell.

1 fails to handle arguments which contain spaces. 2 solves this, but
requires some work. 3 gets the shell to do the work, but the cost is
that you get a lot of other stuff which you may not want
(substitution, redirection, loops, subshells, ...).

--
Glynn Clements <glynn@gclements.plus.com>

Paul Kelly wrote:

> GRASS command parsing from the wxgrass CLI:
>
> Split the command into a list by spaces. I realize that this is a problem
> for any command with spaces in the arguments, but I know of no better way to
> do this in this context outside of making users type any command as a Python
> list. If we made people type ['g.region', 'rast=mymap', 'res=30'] instead of
> g.region rast=mymap res=30, I don't think anyone would want to use the CLI.
>
> If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
> d.* command, send the original command string (not the list) to the
> menuform.py processor (just like the menu would) to create the autogenerated
> properties dialog. After this, the result of hitting "run" is the same as if
> the properties dialog were called from the menu.

I don't get why it needs to do that specific processing - can you not just
run whatever is entered - if a command is run without arguments it will
pop up the GUI dialog anyway. Unless GRASS_UI_TERM is set. I suppose at
the minute it will always be a Tcl/Tk dialog that pops up, but I'm sure it
wouldn't be very hard to change G_parser() to check the GRASS_GUI variable
(or do something similar) and generate the correct autogenerated dialog
corresponding to the GUI that's in use.

If you're running the command from a GUI, it's preferable for the
dialogs to be generated by the GUI process rather than as a standalone
process.

E.g. if you select a command from gis.m's menus, gis.m invokes the
command using the --tcltk and evaluates the output (with certain
procedures overriden, e.g. run_cmd), rather than simply running the
command using --ui.

This has the advantage that the GUI gets to see (and optionally
modify) the arguments which were actually used, so that it can e.g.
record the complete command in a history list. If you just use --ui,
the re-invocation is hidden from the GUI.

--
Glynn Clements <glynn@gclements.plus.com>

On Sat, 16 Jun 2007, Glynn Clements wrote:

Paul Kelly wrote:

If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
d.* command, send the original command string (not the list) to the
menuform.py processor (just like the menu would) to create the autogenerated
properties dialog. After this, the result of hitting "run" is the same as if
the properties dialog were called from the menu.

I don't get why it needs to do that specific processing - can you not just
run whatever is entered - if a command is run without arguments it will
pop up the GUI dialog anyway. Unless GRASS_UI_TERM is set. I suppose at
the minute it will always be a Tcl/Tk dialog that pops up, but I'm sure it
wouldn't be very hard to change G_parser() to check the GRASS_GUI variable
(or do something similar) and generate the correct autogenerated dialog
corresponding to the GUI that's in use.

If you're running the command from a GUI, it's preferable for the
dialogs to be generated by the GUI process rather than as a standalone
process.

E.g. if you select a command from gis.m's menus, gis.m invokes the
command using the --tcltk and evaluates the output (with certain
procedures overriden, e.g. run_cmd), rather than simply running the
command using --ui.

This has the advantage that the GUI gets to see (and optionally
modify) the arguments which were actually used, so that it can e.g.
record the complete command in a history list. If you just use --ui,
the re-invocation is hidden from the GUI.

Ah I see, I think. That sounds pretty important. But it does it mean that tricks the module may do such as checking if there are any command-line arguments before running G_parser() will be ineffective? I was thinking of changing g.mkfontcap to work like that. But I suppose, I could still do that and then it will be OK when run from a script or the command-line. Not so bad for the GUI to add a GUI stage to running the command I suppose - it just means an extra click of "Run".

I didn't realise gis.m worked like this too - might there be any value then in removing the functionality that commands run from the command-line pop up a GUI window when run with no arguments, i.e. behave as if GRASS_UI_TERM was set? Was this behaviour only needed for d.m perhaps? It's not important anyway I suppose. But the point about over-riding the Tcl/Tk GUI box's run_cmd to store this history - that's in the gronsole I think - is important and helps to understand it a lot better, thanks.

Paul

Paul Kelly wrote:

>>> If the command has no arguments (i.e., len(cmdlist) == 0) AND it is NOT a
>>> d.* command, send the original command string (not the list) to the
>>> menuform.py processor (just like the menu would) to create the autogenerated
>>> properties dialog. After this, the result of hitting "run" is the same as if
>>> the properties dialog were called from the menu.
>>
>> I don't get why it needs to do that specific processing - can you not just
>> run whatever is entered - if a command is run without arguments it will
>> pop up the GUI dialog anyway. Unless GRASS_UI_TERM is set. I suppose at
>> the minute it will always be a Tcl/Tk dialog that pops up, but I'm sure it
>> wouldn't be very hard to change G_parser() to check the GRASS_GUI variable
>> (or do something similar) and generate the correct autogenerated dialog
>> corresponding to the GUI that's in use.
>
> If you're running the command from a GUI, it's preferable for the
> dialogs to be generated by the GUI process rather than as a standalone
> process.
>
> E.g. if you select a command from gis.m's menus, gis.m invokes the
> command using the --tcltk and evaluates the output (with certain
> procedures overriden, e.g. run_cmd), rather than simply running the
> command using --ui.
>
> This has the advantage that the GUI gets to see (and optionally
> modify) the arguments which were actually used, so that it can e.g.
> record the complete command in a history list. If you just use --ui,
> the re-invocation is hidden from the GUI.

Ah I see, I think. That sounds pretty important. But it does it mean that
tricks the module may do such as checking if there are any command-line
arguments before running G_parser() will be ineffective?

If you add --ui or --tcltk, an "argc > 1" check will pass, so
G_parser() will be invoked.

Such a check only makes sense if you have no required arguments. If
you have required arguments, the user will either have to enter them
or the command line or in the GUI dialog.

If the defaults for any optional arguments are good, adding a check
means that the user doesn't keep getting prompted for arguments where
the defaults will suffice. E.g. d.erase has the check, so "d.erase" is
equivalent to "d.erase color=white" rather than to "d.erase --ui".

OTOH, if it's unlikely that the user will want to simply accept the
defaults, the check should be omitted.

I didn't realise gis.m worked like this too - might there be any value
then in removing the functionality that commands run from the command-line
pop up a GUI window when run with no arguments, i.e. behave as if
GRASS_UI_TERM was set? Was this behaviour only needed for d.m perhaps?

The current behaviour was intended as an improvement over the
terminal-based Q&A dialogue; it wasn't designed with d.m or gis.m in
mind. The --ui option was added later to allow the "argc>1" check to
be bypassed, so that d.m could display a dialog for commands with such
a check. The --tcltk option was added to allow better integration with
gis.m.

--
Glynn Clements <glynn@gclements.plus.com>