Re: directory separator
Makes sense. I used the function in these gnulib modules that currently do not have a dependency on basename (i believe): exclude, fts, glob, link, progname.I noticed for some, like link, they reimplement the basename functionality (although slightly lacking): https://github.com/coreutils/gnulib/blob/master/lib/link.c#L221-L223 . I am not sure if this intentional to avoid the dependency or just the code was so simple it wasn't pulled in quickly.I know we can limit basename inclusion to windows only although not sure it saves a whole lot. I would need some sort of shim for non-windows operating systems to not use basename or ifdefs and not sure if that really is 'simpler' when basename is essentially the functionality needed. ~mitch (they, them) > >
Re: directory separator
Mitch Capper wrote: > Right now filename has no > dependencies but doing this adds basename. I understand circular > dependencies can be handled, but am I fine to just add a new dependency on > a core module like this? No, it's better if 'filename' stays as a module without dependencies. Therefore, every place where you meant to use LAST_SLASH_IN_PATH, consider adding the dependency to 'basename-lgpl' there. Bruno
Re: directory separator
Not thinking of the bigger picture, rather than looking to replicate the string functionality originally there I should have considered the end goal before evaluating possible gnulib modules. Will update. This does bring me back to part of the original question. Right now filename has no dependencies but doing this adds basename. I understand circular dependencies can be handled, but am I fine to just add a new dependency on a core module like this? ~mitch (they, them) > >
Re: directory separator
Mitch Capper wrote:
> there is no gnulib module for strrpbrk or strrcspn.
Indeed, such functions are non-standard and not in gnulib.
> This was used for
> the LAST_SLASH_IN_PATH macro I added. I temporarily just added a static
> implementation to the header along the lines of:
> ```
> static const char* strrpbrk(const char* s, const char* accept) {
> const char* p = s + strlen(s);
> while (--p >= s) {
> const char* c = accept;
> while (*c) {
> if (*c++ == *p)
> return p;
> ```
Gnulib already has the function last_component, in module basename-lgpl,
for this purpose. Why would you need to add another function
LAST_SLASH_IN_PATH, with the same purpose?
Bruno
Re: directory separator
That was my mistake, missed an r in there which also means there is no
gnulib module for strrpbrk or strrcspn. This was used for
the LAST_SLASH_IN_PATH macro I added. I temporarily just added a static
implementation to the header along the lines of:
```
static const char* strrpbrk(const char* s, const char* accept) {
const char* p = s + strlen(s);
while (--p >= s) {
const char* c = accept;
while (*c) {
if (*c++ == *p)
return p;
```
I am aware of the lookup table option instead although given this isn't for
an actual strrpbrk maybe it would be best to just use a purpose built
function for win32 like
```
static char *LAST_SLASH_IN_PATH(char *str) {
if (!str)
return NULL;
char *last_slash = NULL;
while (*str) {
if (*str == '/' || *str == '\\')
last_slash = str; // Update pointer whenever we find a slash
str++;
}
return last_slash;
}
```
~mitch (they, them)
>
Re: directory separator
Mitch Capper wrote: > I did implement most of the shared path code in lib/filename.h but for > windows I do need strpbrk. Am I ok to just add that as a dependency for > the filename module? You don't even need this dependency, since native Windows has strpbrk and since the doc (doc/posix-functions/strpbrk.texi) does not mention any problem of this function on native Windows. So, just use the function. Bruno
Re: directory separator
> If I have understood it correctly, the best way to handle file names on native > Windows is to > - use '\\' as a directory separator when composing file names, > - allow both '/' and '\\' when inspecting file names. Yes, that is largely what I implemented. There is a platform specific DIR_SEPARATOR used for path building and then an ISSLASH function used for reading (or respective indexof type functions for reading the last ). I did implement most of the shared path code in lib/filename.h but for windows I do need strpbrk. Am I ok to just add that as a dependency for the filename module? I know some of the modules adds deps with conditions like[test $HAVE_FCHOWNAT = 0 || test $REPLACE_FCHOWNAT = 1] but those are more about if that module is required. I could add a conditional like: gl_CONDITIONAL([GL_COND_OBJ_WINDOWS_PATH], [case "$host_os" in cygwin* | mingw* | windows*) true;; *) false;; esac]) as used on some other modules and only add that dep for windows. ~mitch (they, them)
Re: directory separator
Mitch Capper wrote: > *With Path accidental separator detection* > The windows path branch tries to add either forward or backward path > separator support in windows. Clearly \ is also the escape char so it is > possible that there are places that A) windows paths that have escape chars > used will fail, as it is treated as a dir separator (and I don't have \\ > detection etc). B) there are places in code where '/' was being used as a > flag or not a path separator that I mistook for a path separator and now > '\' may have odd effects. We certainly like to see fixes in this area. If I have understood it correctly, the best way to handle file names on native Windows is to - use '\\' as a directory separator when composing file names, - allow both '/' and '\\' when inspecting file names. Bruno
