openvehicles / Open-Vehicle-Monitoring-System-3

Open Vehicle Monitoring System - Version 3
http:///www.openvehicles.com/
Other
599 stars 228 forks source link

microrl quoting escapes & error output #198

Open dexterbg opened 5 years ago

dexterbg commented 5 years ago

Escaping quote chars in quoted arguments does not work, and the error output is not sent to the OvmsWriter:

OVMS# vfs append "a \"little\" test" /sd/test.txt
ERROR:too many tokens or invalid quoting

If done remotely, the command fails silently.

slcasner commented 5 years ago

Backslash quoting is not implemented (and the original microrl had no quoting at all). When I considered how to add quoting, the backslash option seemed a more difficult fit.

You can use single-quotes: vfs append 'a "little" test' /sd/test.txt

slcasner commented 5 years ago

What does "remotely" mean? BufferedShell?

dexterbg commented 5 years ago

Just checked: output on ssh works, so seems to be BufferedShell.

Using single quotes is no option, users can enter arbitrary strings containing both quotes.

slcasner commented 5 years ago

Well, we never promised bash or csh.

slcasner commented 5 years ago

In ovms_webserver.cpp, ExecuteCommand() does:

BufferedShell* bs = new BufferedShell(false, verbosity);

The first argument "false" tells OvmsShell to discard anything microrl prints, such as echoes of the input characters, but also including the error message about invalid quoting. If the correct action would be for microrl to always print error messages, I could fix this by adding a separate (*error_print)() member to microrl_t and always set that one to the Print() function.

slcasner commented 5 years ago

Fixed in 968bedeb74bb6f5fd04cce6b4ba949e2d3f61e3f.

dexterbg commented 5 years ago

Thanks for the error output, Steve. Works in all channels now.

Todo: Escaping quote chars in quoted arguments

slcasner commented 5 years ago

I fixed the part that I believe was a bug. Adding another method of quoting is an enhancement to be prioritized accordingly.