Skip to content

Make it so that li3 shell script still works in directories that have spaces in the names. #450

Merged
merged 1 commit into from May 24, 2012

3 participants

@phuff
phuff commented May 8, 2012

console/li3 doesn't currently work on directories that have spaces in the names. This commit makes it so it still works when there's a space in the directory name.

@davidpersson
Union of RAD member

Are you sure all the quotes are required? Especially the ones on the first line. My guess is that when readlink(1) works without test(1) does so, too. However we shouldn't be guessing ;) Do you think it's possible to come up with a test case for the console wrapper? Maybe one that creates certain directories and cds into them etc. pp.

phuff replied May 8, 2012

So, test(1) was actually the first thing that failed. When I got the quotes around that I made it to the second line :) I would be happy to write a test case, but in order to make it a real test or not radically alter the li3 shell script as it stands (which is nice and simple) the test would have to move the whole li3 framework to a different directory, which seems overboard. I guess I could write a test which copies only the bare essentials... It looks like maybe the lithium.php script would throw a known exception and we could check for it in the shell script... Are there any other sh test cases anywhere? Where would I even put it? :)

Union of RAD member

I don't think this is worth writing a test case for. If it works on OSX, and it works in Travis, it's good enough for me.

Whatever you guys want to do is fine with me. I would have written a test case but it's been a hectic couple of weeks at work :)

@davidpersson
Union of RAD member

Yeah so I guessed wrong... anyway. I think it would not be necessary to move the whole framework for such a test. It would be enough to create some directories below the temporary directory then copy the li3 and probably a mocked lithium.php wrapper there. That would be the minimum to exploit the spaces weakness.

On a total other hand I'd be really excited to see a test for this (maybe using proc_exec() and above techniques) but would vote not to invest too much resources into this as this shell scripting is really an exception when seen in the context of the other framework code.

@davidpersson
Union of RAD member

One way or the other: can you remove the quotes not required than from your patch?

@nateabele nateabele merged commit 0816a2b into UnionOfRAD:dev May 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.