POSIX path separator incompatible with Windows environment

Here is a patch for command.inc, doing some regex magic on the $include string, so it will only split on a : when it is not followed by a \.

After the patch, the $include string will only be split on a colon when it is not followed by a backslash. That means that Windows users can define multiple include paths as follows: $options['i'] = 'C:\drush\drush_make:C:\drush\examples';. This makes the patch backwards compatible with existing drushrc.php files, except for one edge case: when, on a non-windows OS, the first character of the second (or higher) path is a backslash, the string would not be split.

[edit:]
@greg: It should not matter if windows users end their paths with a \, because the regex splits the string on all colons that are not followed by a backslash. Preceding backslashes have no effect (assuming that I wrote the regex correctly :-)). I'm not a big fan of regex-based solutions myself, but changing the separator would break existing drushrc.php files, which is not ideal either.

It's been a while since the last post here... Time to make a choice and fix this? I'd be happy to supply another patch if necessary.

So... here are two patches. One changes the colon delimiter to a comma delimiter - probably the simplest patch I've written so far :-)

The second patch is a slightly simplified version of the regex-patch from #1. Personally, I don't really have a preference for either solution, but dismissing this solution because it's 'magic' and 'complex' doesn't do it justice. The regex :(?!\\) means: match a colon ( : ) when followed by ( (? ) ) not ( ! ) a backslash (escaped as \\ ). I tend to find regex difficult myself too, but seriously, they don't get much easier than this one.

Anyway, I hope this helps to get one of these patches committed. Thanks.

Ok, I didn't know about the path separator constant, thanks. I'll give it a shot later today.

Here's a new patch, now targeting all occurrences of path separators I could find. The include path (which started this issue) and rsync have already been mentioned, but also php-script accepts a list of paths. This patch makes them use the PATH_SEPARATOR constant. I also updated the descriptions in core_drush_command() and the inline docs in example.drushrc.php.

I'm not sure what this piece of code does (around line 501 of core.drush.inc in function _core_path_aliases()):

      // If the 'project' parameter was specified, then search
      // for a project (or a few) and add its path to the path list
      if (!empty($project)) {
        foreach(explode(',', $project) as $target) {
          $path = drush_core_find_project_path($target);
          if(isset($path)) {
            $paths['%' . $target] = $path;
          }
        }
      }

It seems similar to the other path separator occurrences, but I couldn't figure out what the $project option does, so I left it as is.

Jonhattan, did you test the patch yourself on Windows? I now realize (oops!) that I forgot to mention something important when I posted the patch last night: I wrote it on my home linux machine and I have not tested it on Windows yet. I'll do some proper Windows testing asap, unless someone already did that.

Please set the issue back to RTBC if I'm being too cautious here.

Here's a new patch changing one more line of documentation in example.drushrc.php.

I have tested the --include and --script-path options on windows and the ; separator works fine. We do not use drush rsync here at the office(yet) so I wasn't able to test that. If someone can test rsync with this patch (if not on Windows, then at least in a unix environment), I think we can call this RTBC.

Forgot the attachment.

I agree. As long as this doesn't break anything on unix, this patch is de facto an improvement, even if it wouldn't work for all Windows users.