muesli / go-app-paths

Lets you retrieve platform-specific paths (like directories for app-data, cache, config, and logs)
MIT License
201 stars 5 forks source link

Expose DataDir() #4

Open LaurenceGA opened 4 years ago

LaurenceGA commented 4 years ago

I like this library! I just noticed that CacheDir is exposed for a scope, but there is not equivalent DataDir().

Is there any reason not to expose an equivalent function like this? Seems like it would be very easy to do.

:memo:

muesli commented 4 years ago

You can just access the first element of DataDirs instead. Unlike CacheDir it's a priority-sorted slice of directories, since there may be multiple valid data directories on a system.

LaurenceGA commented 4 years ago

I understand that. That's pretty much what I did. But I essentially just wrote a wrapper method that calls DataDirs() and returns the first element.

My use case is essentially that I want the path to the default data directory. It seems like a similar use case is accommodated for by DataPath(string). So it would just be this method without appending a filename.

It's not a big deal, I just think it should be considered for the sake of making the interface easier to use.

muesli commented 4 years ago

Maybe I'm not understanding your use-case correctly? What does your wrapper look like?

What I usually do is call DataDirs()[0] instead of DataDir(). I agree it's a few more characters to type out, but I'm not sure that really warrants exposing another function publicly? I'll have to think about it. I'd at least like to avoid the naming confusion between the singular and plural forms.

We could at least improve the documentation here and mention that DataDirs() is guaranteed to always return at least one item in the slice.

LaurenceGA commented 4 years ago

I just wrote the wrapper like:

func DataDir(scope *gap.Scope) (string, error) {
    dataDirs, err := scope.DataDirs()
    if err != nil {
        return "", err
    }

    if len(dataDirs) == 0 {
        return "", errors.New("couldn't find a single data directory")
    }

    return dataDirs[0], nil
}

I suppose checking the length isn't actually necessary though... I feel I'd rather encapsulate that assumption in the library, but :man_shrugging:

I see why it's currently how it is, just when I first used it I wanted any easy way to access the default data dir. Perhaps this is best solved by just a documentation update, not sure...