Re: Fix for HAVE_DOS_PATHS build on cygwin

[Christopher Faylor]

On Thu, Aug 17, 2006 at 12:38:29PM -0400, Bill Hoffman wrote:
>At 11:42 AM 8/17/2006, Christopher Faylor wrote:
>>On Thu, Aug 17, 2006 at 10:23:35AM -0400, Bill Hoffman wrote:
>>
>>
>>>                    if (p[1] != '\\' && p[1] != '\''
>>>                        && !isspace ((unsigned char)p[1])
>>>!                       && strchr (sh_chars, p[1]) == 0)
>>>                      /* back up one notch, to copy the backslash */
>>>                      --p;
>>
>>I don't think this is right.  The comment above this line refers to operations
>>on "Unixy shells".  I think that just adding something like:
>>
>>#ifdef HAVE_DOS_PATHS
>>static const char *sh_chars_sh = sh_chars;
>>#endif
>>
>>in the "#else /* must be UNIX-ish */" clause would probably handle this.
>
>That would work, but it sort of seems odd that sh_chars_sh would be directly
>referenced anywhere.  It looks to me like sh_chars should be used in all
>places, so that sh_chars_sh and sh_chars_dos can be set at run time.
>I guess it is a style thing, but this is the only place sh_chars_sh is
>referenced outside of an initialization for sh_chars.  So, it seemed out
>of place to me.  

Read the comment I mentioned.  It explains why this is sh_chars_sh rather
than sh_chars.

It does make me wonder what kind of quoting problems we'd see with
HAVE_DOS_PATHS, though.  On cygwin, I wouldn't want every backslash to
be interpreted as part of a path name "just in case".

>>>  /* Handle other OSs.  */
>>>! #if defined(HAVE_DOS_PATHS) && !defined(__CYGWIN__)
>>>  # define PATH_SEPARATOR_CHAR ';'
>>>  #elif defined(VMS)
>>>  # define PATH_SEPARATOR_CHAR ','
>>
>>This is probably too big a change for this particular problem (and maybe
>>it's already been discussed to death) but I don't see why this setting
>>couldn't be in configure.in, too.  Then MinGW could set
>>PATH_SEPARATOR_CHAR to ';', VMS could set it to ',' (shouldn't that be
>>'.'?), and Cygwin and every other "UNIX system" could just use the default.
>
>Sure, this seems to produce the correct behavior.  If you want
>to make it a configure.in thing, then provide a patch.  I was just going
>along with the existing style of the code in this case. 

I'm not a make maintainer and don't really care about this one way or
the other.  I was just pointing out that I thought it made sense to do
things this way.  Personally, I think that things like "#ifdef
__<OSNAME>__" to code, should be avoided although I don't see a way
around it in this case, if the current way of handling things is
maintained.

cgf