Re: [Edbrowse-dev] wordexp again

[Adam Thompson <arthompson1990@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4031 bytes --]

On Sat, Apr 18, 2015 at 12:18:00AM -0400, Karl Dahlke wrote:
> Well a very smart person told me to write a 10 line C program. Here it is.
> 
> #include <stdio.h>
> #include <wordexp.h>
> main(int argc, char **argv)
> {
>         wordexp_t w;
> char line[80];
> while(gets(line)) {
> wordexp(line, &w, 0);
> printf("%s\n", w.we_wordv[0]);
>         wordfree(&w);
> }
> }
> 
> Create the two files abc and ab\d
> Run the program and type in various strings.
> Here is input and output.
> 
> jkl jkl
> \\ \
> \\\\ \\
> ab? abc
> ab\\e ab\e
> ab\\* ab\*  (should be ab\d)
> ab\\$HOME ab\/home/eklhad
> $HOME\\ab /home/eklhad/\ab
> \\'j' \j
> ab\\\\* ab\d
> 
> So backslashes are crunched once, and \' becomes ', and \| becomes |,
> and so on as $variables are replaced.
> Then, the next pass, globbing, if globbing occurs,
> or if you want it to occur, then \\ becomes \ once again.
> However \' does not become ', only \\ is crunched.
> This second crunching is the bug, and there is really no way around it.
> That's why ab\\\\* expands the way ab\\* should.
> 
> If I have the energy, and I'm not sure if I do,
> but if I do, I believe the right answer is to write my own wordexp function
> that does the following:
> 
> pass 1
> ignore ' " | () [] ; \ blah blah blah,
> no nasty side effects of calling this function.
> No confusion, and no reason not to run it all the time.
> don't have to start with a ` to invoke it, just run it because
> it doesn't do anything weird.
> Replace $var with its environment value,
> and maybe even ${var}.
> Let's be honest, this software is easy, and entirely portable.
> 
> pass 2
> Call glob() to expand any shell wild cards.
> This is the hard part, so let the library do that,
> but glob doesn't screw up other characters in the string.
> 
> That's what I will sleep on tonight,
> and see how I feel about it in the morning.

I notice you miss out the ~whatever expansion here,
which is a missing feature of this. As I said,
I think the ` inverting probably fixes most of this,
with the caveat about the cd command.
I also think we shouldn't be using glob to scan a directory,
we should be using scandir and alphasort,
no need to append / or /* to strings in this case and no other oddness possibly
caused by odd directory names (I've got a suspician there's another escaping
bug with this code somewhere).
I'm also not entirely sure I'd mind having command substitution enabled,
particularly as I use this a lot in file name expansions now.
I definitely think we need to talk about this,
particularly now we have a more or less fixed version of edbrowse.
What the last couple of days have shown is that making undiscussed changes is
just going to get circular due to differing user requirements and opinions.
In the environment inwhich I use Edbrowse,
the more file name expansion features I can have enabled the better,
however other people aren't as comfrtable in the shell.
I'd quite like a sort of hybrid approach actually which keeps the ` character
for full wordexp (with command substitution as well)
but may be some sort of glob (plus ~ expansion) without it.
If you're specifying environment variables then your probably in the realm of
the shell, and then you may as well have all the power that implies.

@Karl: I think one of the other issues here is that you've been using the code
the way you wrote it for years, whereas I'm coming to the env var expansion
stuff fairly fresh and based on years of using the shell.
That gives two different perspectives on what the "right"
thing is in this case.
As for the bug in wordexp, I need to read through the posix standard to se how it
handles escaping to see if this is a genuine bug or just us not understanding
how wordexp is supposed to work with quoting.
At the end of the day, people claim wordexp's expansion is the same as the
shell, but it's not the same function and thus the expansions are slightly different.

Cheers,
Adam.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]