murex
murex copied to clipboard
Backslashes and code comments
exec echo "one" \
"two" \
"three"\
"four"\ # test
"five" \ # test
"six"
prints one two three four five six
. I didn't check in detail, but those additional spaces are very likely some additional command line arguments that should not be there. This is causing interference with the application's argument parser.
Edit: Oh wow, that's sneaky: apparently some of those spaces are zero-width and only visible with monospace fonts?
Ack, I've clearly cocked something up in the parser there and somehow also failed to add tests around that particular condition. I'll look into this and hopefully have a fix in the next release.
Apologies for the inconvenience there.
Btw I found a workaround: simply put the backslash behind the comment to make things work again:
exec echo "one" \
"two" \
"three" \
"four" # test \
"five" # test \
"six"
Not sure if that is intended though
It wasn't and you've potentially unearthed another bug there.
I'll give the parser some love over the next few weeks. If truth be told, it's some of the oldest code in the project and I had intended to rewrite it at some point anyway because it's just a simple single pass compiler and thus doesn't handle nested subshells correctly either. Which was fine in the early POC stages of the shell but we're a long way beyond that now.
I'll probably do this as a two stage release though:
- first stage is to do some TDD and put a quick hack in to get those tests working.
- Then I'll start on the rewrite as I should have some confidence that any rewrite wouldn't introduce new regression bugs
Thanks as always for raising these issues.
Written a patch to resolve the slash before a comment problem (pushed to develop
). It's a very experimental patch and certainly doesn't address the full parser rewrite. But it's hopefully enough to close off this issue, once it hits master
.
The interesting thing is, during my testing I noticed that neither Bash nor Zsh actually support either scenario. But I think we can do better. So now both of the following tests pass in murex:
{
Block: `out: one\
two \
three\ # test
four \ # test
five
out: six`,
Stdout: "one two three four five\nsix\n",
},
{
Block: `out: one\
two \
three # test \
four # test \
five
out: six`,
Stdout: "one two three four five\nsix\n",
},
Thank you.
during my testing I noticed that neither Bash nor Zsh actually support either scenario. But I think we can do better
Yes. There is a StackOverflow question on that topic (can't find it right now), the accepted answer is a really ugly hack.
So now both of the following tests pass in murex
Hm, does this mean that I can add the backslash either before the comment symbol or before the end of line, and both will work?
Hm, does this mean that I can add the backslash either before the comment symbol or before the end of line, and both will work?
In theory, yes.
My thinking is that, as a developer, formatting the code is an activity that doesn't generate value aside readability (ie it's not part of the problem solving, it's just tidying up the aesthetics). So in supporting both placements of the escape character murex is removing any cognitive overhead nor research needed for performing that activity thus allowing the developer to move quickly back onto the task that does add value (ie the logic of the code rather than the formatting of it).
The reason I say "in theory" is because I need to write a few more tests to ensure there aren't any weird parsing glitches created by this patch plus the interactive terminal may behave slightly different too since it is parsed line by line. It was late then I pushed this commit through :) I hope to get more thorough tests in place over the next few days.
Current patch has been into master
. Leaving this issue open for now as I still want to write more tests plus backport the change to the interactive terminal. However v2.4 was already looking like a large release so didn't want to hold that up any longer. I might sneak a few updates to this issue in point releases rather than waiting for a v2.5 release.