Opened 11 years ago

Closed 10 years ago

#528 closed defect (fixed)

wxGUI cmd: r.mapcalc fails (quoting + args with spaces)

Reported by: neteler Owned by: martinl
Priority: major Milestone: 6.4.0
Component: wxGUI Version: svn-develbranch6
Keywords: Cc: grass-dev@…
CPU: All Platform: All

Description

Running in Cmd> of wxGUI the following commands (NC data set):

r.buffer lakes out=lakes_buff dist=60,120,240,500 --o
r.mapcalc "developed_lake=if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())"

leads to

r.mapcalc "developed_lake=if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())"
syntax error, unexpected $end, expecting '='
Parse error

Maybe the " chars are not protected?

Markus

Unrelated idea: make the > of Cmd > clickable to clear the line?

Attachments (1)

shlex_try1.diff (1.5 KB) - added by hamish 10 years ago.
non-functional patch using shlex.split() instead of split(' ') [attach to the correct ticket this time]

Download all attachments as: .zip

Change History (23)

comment:1 Changed 11 years ago by neteler

Component: defaultwxGUI
CPU: UnspecifiedAll
Platform: UnspecifiedAll
Version: unspecifiedsvn-develbranch6

comment:2 in reply to:  description ; Changed 11 years ago by martinl

Replying to neteler:

> r.mapcalc "developed_lake=if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())"
> syntax error, unexpected $end, expecting '='
> Parse error

Maybe the " chars are not protected?

yes, anyway it should without quotation marks

r.mapcalc developed_lake = if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())

Martin

comment:3 in reply to:  description Changed 11 years ago by martinl

Replying to neteler:

Unrelated idea: make the > of Cmd > clickable to clear the line?

Done in r36354 (trunk) and r36355 (devbr6).

Martin

comment:4 in reply to:  2 Changed 11 years ago by hellik

Replying to martinl:

Replying to neteler:

> r.mapcalc "developed_lake=if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())"
> syntax error, unexpected $end, expecting '='
> Parse error

Maybe the " chars are not protected?

yes, anyway it should without quotation marks

r.mapcalc developed_lake = if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())

Martin

i tried it on the wx-cmd-line (osgeo4w, win vista) without quotation marks and the calculation is done.

maybe there could be a hint for this in http://skagit.meas.ncsu.edu/~helena/grasswork/grassbookdat07/ncexternal/grasstestrast.txt

helmut

comment:5 in reply to:  2 ; Changed 11 years ago by neteler

Replying to martinl:

Replying to neteler:

> r.mapcalc "developed_lake=if(landuse96_28m==1 || landuse96_28m==2, lakes_buff, null())"
> syntax error, unexpected $end, expecting '='
> Parse error

Maybe the " chars are not protected?

yes, anyway it should without quotation marks

This would break compatibility to the common command line (hence, in all books, tutorials, courses etc this hint would be required which is rather cluttering stuff). If technically possible (and time permits) I hope that quotes can be maintained also in the wx-cmd line.

Markus

comment:6 in reply to:  5 Changed 11 years ago by glynn

Priority: majorcritical

Replying to neteler:

Maybe the " chars are not protected?

More likely, they aren't *interpreted*, e.g. something is splitting the command line into arguments, treating every spaces as a separator.

yes, anyway it should without quotation marks

Not in 7.0 it won't, nor will that work for e.g. title=... or filename arguments which need to accept strings containing spaces.

This would break compatibility to the common command line (hence, in all books, tutorials, courses etc this hint would be required which is rather cluttering stuff). If technically possible (and time permits) I hope that quotes can be maintained also in the wx-cmd line.

If the GUI is to provide a command line, it needs to actually support the basic features of any command-line interface, e.g. the ability to have spaces, quotes etc within arguments.

If it simply passing the command on to the OS, it should probably just pass shell=True to Popen() rather than trying to parse it itself. If it needs to know how the command is parsed, I suggest writing a small helper program which writes its argv[] to stdout, separated by NULs, then have the GUI invoke that program via the shell and parse its output.

comment:7 Changed 11 years ago by cmbarton

Currently, the command line is passing the command to the OS, with some simple parsing to 1) check for valid GRASS commands and 2) to get it into Popen(). A full terminal is possible via Python and wxPython, but it will take 1) some considerable reprogramming and 2) some space (maybe could replace the output window). It's probably a very good idea but I would prioritize it below some other more critical issues to be fixed.

We need more than Martin and I on the GUI team to help explore some of these good ideas and fix existing bugs (e.g., the attribute manager is now broken for Mac).

This seems more of an enhancement request than a defect.

Michael

comment:8 Changed 11 years ago by cmbarton

Forgot to mention. The ">" is just text, but a clear button could be added. Another good idea.

Michael

comment:9 in reply to:  7 Changed 11 years ago by glynn

Replying to cmbarton:

This seems more of an enhancement request than a defect.

The inability to have spaces within arguments is a defect, and a rather fundamental one.

If the GUI doesn't need to understand the command(s), the string should be passed verbatim to Popen(..., shell=True). But this will allow arbitrary shell commands, including redirection, pipelines, for/if/while, etc.

If you need to parse a string into program and argv[], consider using shlex.split():

http://docs.python.org/library/shlex.html

comment:10 Changed 11 years ago by cmbarton

I'm not sure we're talking about exactly the same thing.

Currently the very simple command processor does not require quotes around the main r.mapcalc argument. This is also the case in other settings, including the TclTk? command processor. Quotes are not required around the main r.mapcalc argument in the terminal on my Mac either. That was the original issue. I don't see this, in and of itself as a defect--though I suppose others might disagree.

Spaces, in and of themselves are not a problem in the command processor. The following works fine, for example

r.mapcalc newmap = 5

Note the lack of quotes and the spaces on either side of the "=".

I think the command processor also allows strings with spaces surrounded by quotes within arguments if a command requires them, but I would need to test this on a particular set of cases. I currently can't think of an appropriate test, but I'm sure there are such cases. If it does not allow spaces within an argument (as allowed by a GRASS command), this does need to be fixed if possible so that any GRASS command can be parsed correctly.

Michael

comment:11 in reply to:  10 ; Changed 11 years ago by glynn

Replying to cmbarton:

I'm not sure we're talking about exactly the same thing.

Currently the very simple command processor does not require quotes around the main r.mapcalc argument. This is also the case in other settings, including the TclTk? command processor. Quotes are not required around the main r.mapcalc argument in the terminal on my Mac either. That was the original issue. I don't see this, in and of itself as a defect--though I suppose others might disagree.

This isn't true in 7.0, where r.mapcalc uses the parser. The 6.x r.mapcalc is a special case, as it simply concatenates all of its arguments, separated by spaces, which eliminates the need for quoting. Any command which uses the parser requires quoting (or escaping) if an argument contains spaces.

Spaces, in and of themselves are not a problem in the command processor. The following works fine, for example

r.mapcalc newmap = 5

Note the lack of quotes and the spaces on either side of the "=".

r.mapcalc is a poor example, for the reason given above.

I think the command processor also allows strings with spaces surrounded by quotes within arguments if a command requires them,

The original report suggests otherwise. That is the real problem here. The fact that the 6.x r.mapcalc provides a workaround doesn't help in general.

but I would need to test this on a particular set of cases. I currently can't think of an appropriate test, but I'm sure there are such cases.

Choose any command which accepts a filename argument, and pass a filename which contains spaces.

comment:12 in reply to:  11 Changed 11 years ago by hellik

Replying to glynn:

Replying to cmbarton:

The original report suggests otherwise. That is the real problem here. The fact that the 6.x r.mapcalc provides a workaround doesn't help in general.

but I would need to test this on a particular set of cases. I currently can't think of an appropriate test, but I'm sure there are such cases.

Choose any command which accepts a filename argument, and pass a filename which contains spaces.

maybe related?

platform grass64-rc3-3 (osgeo4w) win vista with wx-gui

command taken from the short tutorial in http://www.grassbook.org/examples3rd_chapter6.php

copied from the website to the wx-gui command-line: v.extract stopsall out=stops1 where="ROUTES LIKE '%1%' AND ROUTES NOT LIKE '%11%'"

following error-message:

v.extract stopsall out=stops1 where="ROUTES LIKE '%1%' AND ROUTES NOT LIKE '%11%'" Sorry <LIKE> is not a valid option Sorry <'%1%'> is not a valid option Sorry <AND> is not a valid option Sorry <ROUTES> is not a valid option Sorry <NOT> is not a valid option Sorry <LIKE> is not a valid option Sorry <'%11%'"> is not a valid option Description:

Selects vector objects from an existing vector map and

creates a new map containing only the selected objects. Keywords:

vector, extract

Usage:

v.extract [-dtr] input=name output=name [...]

the working command in the v.extract-gui looks like:

v.extract input=stopsall@wolfline_lrs output=stops1 type=point where=ROUTES LIKE '%1%' AND ROUTES NOT LIKE '%11%'

25 categories loaded from table <stopsall> Extracting features... Building topology for vector map <stops1>... Registering primitives... 25 primitives registered [...]

but if i copy the working command of the v.extract-gui to the wx-gui-cmd, the i get again the first error message with

v.extract input=stopsall@wolfline_lrs output=stops1 type=point where=ROUTES LIKE '%1%' AND ROUTES NOT LIKE '%11%' Sorry <LIKE> is not a valid option [...]

Helmut

comment:13 Changed 10 years ago by hamish

Summary: wxGUI cmd: r.mapcalc failswxGUI cmd: r.mapcalc fails (quoting + args with spaces)

see also bug #604 for the GIS.m equivalent.

spearfish test case:

v.db.select roads where="label ~ 'highway'"

Hamish

comment:14 Changed 10 years ago by hamish

Hi,

I just added a patch to this ticket (vs devbr6) which roughly uses shlex.split(command) to parse the command string in a shell-like way. It doesn't actually work but I think it gets close. something like the command = ' '.join(command) from the Xterm fn is happening so it just tries to execute the first letter. Hopefully a more learned pythoner can quickly finish the job from here.

Hamish

Changed 10 years ago by hamish

Attachment: shlex_try1.diff added

non-functional patch using shlex.split() instead of split(' ') [attach to the correct ticket this time]

comment:15 in reply to:  14 ; Changed 10 years ago by martinl

Replying to hamish:

I just added a patch to this ticket (vs devbr6) which roughly uses shlex.split(command) to parse the command string in a shell-like way. It doesn't actually work but I think it gets close. something like the command = ' '.join(command) from the Xterm fn is happening so it just tries to execute the first letter. Hopefully a more learned pythoner can quickly finish the job from here.

Modified path applied in trunk (r37395), devbr6 (r37396) and relbr64 (r37397).

Testing welcomed ...

Martin

comment:16 in reply to:  15 ; Changed 10 years ago by hamish

Replying to martinl:

Modified path applied in trunk (r37395), devbr6 (r37396) and relbr64 (r37397).

thanks.

Testing welcomed ...

  • autoparsing is nice in theory but slows things down and breaks for non-grass commands. e.g. "ls -l" or "gdalinfo --version"
  • left and right arrows don't work to move the cursor
in the terminal:
ls: unrecognised option `--interface-description'
Try `ls --help' for more information.
...

in the Command output tab:
...
self._module = menuform.GUI().ParseInterface(cmd = cmd)
  File "/usr/src/grass/svn/develbranch_6/dist.i686-pc-linux-
gnu/etc/wxpython/gui_modules/menuform.py", line 1809, in
ParseInterface

xml.sax.parseString(getInterfaceDescription(cmd[0]).decode(e
nc).encode("utf-8"),
  File "/usr/src/grass/svn/develbranch_6/dist.i686-pc-linux-
gnu/etc/wxpython/gui_modules/menuform.py", line 1769, in
getInterfaceDescription

raise IOError, _("Unable to fetch interface description for
command '%s'.") % cmd
IOError
:
Unable to fetch interface description for command 'ls'.

gdalinfo --version:

  File "/usr/lib/python2.5/xml/sax/xmlreader.py", line 123,
in parse

self.feed(buffer)
  File "/usr/lib/python2.5/xml/sax/expatreader.py", line
211, in feed

self._err_handler.fatalError(exc)
  File "/usr/lib/python2.5/xml/sax/handler.py", line 38, in
fatalError

raise exception
xml.sax._exceptions
.
SAXParseException
:
<unknown>:1:0: syntax error

comment:17 Changed 10 years ago by hamish

also when typing in

v.db.select roads where="label ~ ...

you get an error:

...
raw = self.read_token()
  File "/usr/lib/python2.5/shlex.py", line 172, in
read_token

raise ValueError, "No closing quotation"
ValueError
:
No closing quotation

can auto-completion be restricted to only run if you press the tab key?

Hamish

comment:18 in reply to:  16 Changed 10 years ago by martinl

Replying to hamish:

  • autoparsing is nice in theory but slows things down and breaks for non-grass commands. e.g. "ls -l" or "gdalinfo --version"
  • left and right arrows don't work to move the cursor

Note that autocomplete propmt is in very early stage of development. Autoparsing should be reduced only to GRASS commands.

Martin

comment:19 Changed 10 years ago by martinl

Cc: grass-dev@… added
Owner: changed from grass-dev@… to martinl
Priority: criticalmajor
Status: newassigned

Probably we should create new ticket since the original bug has been fixed.

comment:20 in reply to:  16 Changed 10 years ago by martinl

Replying to hamish:

  • autoparsing is nice in theory but slows things down and breaks for non-grass commands. e.g. "ls -l" or "gdalinfo --version"
  • left and right arrows don't work to move the cursor

hopefully fixed in r37423 (trunk) and r37424 (devbr6).

Martin

comment:21 Changed 10 years ago by martinl

I took liberty to close the ticket, please reopen if needed.

Martin

comment:22 Changed 10 years ago by martinl

Resolution: fixed
Status: assignedclosed

Ops, now closed.

Note: See TracTickets for help on using tickets.