On Fri, Apr 17, 2015 at 10:58:26PM +0100, Adam Thompson wrote:
> On Fri, Apr 17, 2015 at 10:34:05PM +0100, Adam Thompson wrote:
> > > By the way, removing quick check had nothing to do with the bug
> > > that I posted at the top of this thread.
> > > You stil can't edit a directory with backslash in the name,
> > > and I think that is a bug in wordexp that we can't get around.
> > > That's maybe another reason to go back to my home grown code.
> 
> No, it's a bug in our code. I've checked and we've got a couple of interesting
> things happening. With wordexp enabled, we're duble expanding the filename,
> once before we pass it to sortedDirList, and once in nextScanFile.
> Without wordexp expansion, we don't expand the file name before we get to
> nextScanFile, but then the single \ is swallowed by wordexp so the glob is incorrect.
> I should've spotted this when we first switched to using wordexp because the
> call in nextScanFile really should be to glob rather than wordexp.
> Better still, I think we just shouldn't have switched nextScanFile to wordexp
> since we just want a directory listing in this case.

In the case of sortedDirList we should be using scandir with the alphasort
function as a comparison function (then loading into your line map probably),
for the NextScanFile function (used in fetchmail.c I think)
we probably want to go back to readdir.
I'm happy to put together a fix for this tomorrow.

> I've also tried out the inverted ` character and that seems to make much more
> sense in terms of usable behaviour.

Any thoughts? I think this makes more sense to me since I don't have to be
trying to work out what and when I need to escape things, i.e.
with the "quick check" if I use ~/ then I have to shell escape everything,
or if I use $ then I have to escape everything.
Whereas with the ` character acting to *enable* expansion,
then if I specify ` at the start of the filename then I know I need to shell
escape everything because I'm getting wordexp.
The change I made also had a \ escape for ` in the rare case that the leading ` is required.
If you're happy with this then I'll test and push this change.

Cheers,
Adam.