search

qw3rtty / neix

neix - a RSS/Atom feed reader for your terminal.
GNU General Public License v3.0
200 stars 12 forks source link

Added XDG Base Directory support #13

Closed Klesomik closed 3 years ago

Klesomik commented 3 years ago

Hello!

Maybe it is better to replace hardcoded path "$HOME/.config/neix/" to "$XDG_CONFIG_HOME/neix/"?

qw3rtty commented 3 years ago

Thanks for opening this PR. I will check it out.

qw3rtty commented 3 years ago

I got problems to build it on a Mac. I provide tomorrow more informations.

qw3rtty commented 3 years ago 
CMake Warning at /usr/local/Cellar/cmake/3.17.2/share/cmake/Modules/Platform/Darwin-Initialize.cmake:286 (message):
  Ignoring CMAKE_OSX_SYSROOT value:

CMake Warning at /usr/local/Cellar/cmake/3.17.2/share/cmake/Modules/Platform/Darwin-Initialize.cmake:286 (message):
  Ignoring CMAKE_OSX_SYSROOT value:

This is what I got on an Mac. Can you provide a fix for this?

qw3rtty commented 3 years ago

@Klesomik Did you had time to take a look on the build issues on a Mac?

dhgutteridge commented 3 years ago

Adding my two cents here. The notion of using XDG environment variables is more a Linuxism than something that can be assumed to be universally applicable to other OSes. The existing patch set isn't adequate: since you can't safely assume that $XDG_CONFIG_HOME is defined everywhere, there would also have to be a check to ensure it was picked up, and another fallback applied if not (presumably $HOME).

I'm also mentioning this because this PR may partly conflict with others that I'm submitting. Since my PR #19 was integrated, there's really no longer a need for the MAIN_CONFIG_PATH and FEED_CONFIG_PATH definitions in CMakeLists.txt. I'd argue that the other entries being changed in the CMakeLists.txt file are also not warranted. They don't necessarily work as-is as intended (I think) for more than one reason, primarily because they assume the build environment is the same as the runtime, which is not the case for any downstream OSes integrating this software. Changing those to expect $XDG_CONFIG_HOME wouldn't be helpful in that context, as builds may occur in sandboxed and sanitized environments. (I'm involved in cross-platform software packaging, for pkgsrc.org, where we're providing neix binaries for NetBSD, macOS, Solaris derivates, Linux, and more.)

So, I don't think it necessarily shouldn't try for $XDG_CONFIG_HOME if someone really wants that, but I think at minimum that's an addition, not a replacement. Not my decision, of course.

qw3rtty commented 3 years ago

@dhgutteridge Thanks for your detailed comment! It seems pretty clear to me.

Using XDG environment variables is not a good idea for neix. Because it will be used on different Unix systems like MacOS, FreeBSD and more.

@Klesomik Thanks for your work and time but I will not merge this pull-request.