-
Notifications
You must be signed in to change notification settings - Fork 92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a single standard for config/cache/state files #24
Comments
This is indeed inconsistent and quite annoying: something I really wanted to improve. As soon I have enough time I'll work on this one too. |
@tobbez Done: not yet merged into master (still in its own branch). |
@neitsab Sorry if I bother you again. Maybe it would be a good idea to mention it in the pkgbuild install file. |
It would be better to remove the fallback completely, since it would mean less code. It would also make support easier because it would make sure there's always only a single location were config/cache/state would be stored. The workarounds for older versions of pyxdg should also be removed (the almost 4 years since 0.25 was released should be enough for it to be available almost everywhere). The changes mentioned above would leave us with this diff for diff --git a/morituri/common/directory.py b/morituri/common/directory.py
index 47aac11..7af0df3 100644
--- a/morituri/common/directory.py
+++ b/morituri/common/directory.py
@@ -22,34 +22,21 @@
import os
+from xdg import BaseDirectory
+
from morituri.common import log
class Directory(log.Loggable):
-
def getConfig(self):
- try:
- from xdg import BaseDirectory
- directory = BaseDirectory.save_config_path('morituri')
- path = os.path.join(directory, 'morituri.conf')
- self.info('Using XDG, configuration file is %s' % path)
- except ImportError:
- path = os.path.join(os.path.expanduser('~'), '.moriturirc')
- self.info('Not using XDG, configuration file is %s' % path)
+ directory = BaseDirectory.save_config_path('whipper')
+ path = os.path.join(directory, 'whipper.conf')
+ self.info('Configuration file is %s' % path)
return path
-
def getCache(self, name=None):
- try:
- from xdg import BaseDirectory
- path = BaseDirectory.save_cache_path('morituri')
- self.info('Using XDG, cache directory is %s' % path)
- except (ImportError, AttributeError):
- # save_cache_path was added in pyxdg 0.25
- path = os.path.join(os.path.expanduser('~'), '.morituri', 'cache')
- if not os.path.exists(path):
- os.makedirs(path)
- self.info('Not using XDG, cache directory is %s' % path)
+ path = BaseDirectory.save_cache_path('whipper')
+ self.info('Cache directory is %s' % path)
if name:
path = os.path.join(path, name)
@@ -59,25 +46,12 @@ class Directory(log.Loggable):
return path
def getReadCaches(self, name=None):
- paths = []
-
- try:
- from xdg import BaseDirectory
- path = BaseDirectory.save_cache_path('morituri')
- self.info('For XDG, read cache directory is %s' % path)
- paths.append(path)
- except (ImportError, AttributeError):
- # save_cache_path was added in pyxdg 0.21
- pass
-
- path = os.path.join(os.path.expanduser('~'), '.morituri', 'cache')
- if os.path.exists(path):
- self.info('From before XDG, read cache directory is %s' % path)
- paths.append(path)
+ path = BaseDirectory.save_cache_path('whipper')
+ self.info('Read cache directory is %s' % path)
if name:
- paths = [os.path.join(p, name) for p in paths]
-
- return paths
-
+ path = os.path.join(path, name)
+ if not os.path.exists(path):
+ os.makedirs(path)
+ return [path] which would leave us with this (header omitted): import os
from xdg import BaseDirectory
from morituri.common import log
class Directory(log.Loggable):
def getConfig(self):
directory = BaseDirectory.save_config_path('whipper')
path = os.path.join(directory, 'whipper.conf')
self.info('Configuration file is %s' % path)
return path
def getCache(self, name=None):
path = BaseDirectory.save_cache_path('whipper')
self.info('Cache directory is %s' % path)
if name:
path = os.path.join(path, name)
if not os.path.exists(path):
os.makedirs(path)
return path
def getReadCaches(self, name=None):
path = BaseDirectory.save_cache_path('whipper')
self.info('Read cache directory is %s' % path)
if name:
path = os.path.join(path, name)
if not os.path.exists(path):
os.makedirs(path)
return [path] |
@tobbez |
Why not unconditionally depend on PyXDG, instead of re-implementing parts of it? PyXDG is tested, stable, small, available everywhere, so I see no reason to against just using it. Apart from that, there's superfluous code left in |
@JoeLametta No problem for adding a warning about backwards-incompatible changes in the install file. Could you tag this release so that we have a precise version to link against? |
Now whipper always follows XDG specifications. All changes were documented into the README file.
PyXDG is simply not needed in this case. If we rewrite the Thanks, will do once I've finished working on this branch. |
I've just dropped the Thanks |
* Address issue #24 * Remove getReadCaches() & replace it with getCache() Now whipper always follows XDG specifications. All changes were documented into the README file. The changes introduced here aren't backward compatible so, after updating whipper, you may need to configure it again (old config file are retained, though).
@JoeLametta Thanks for the tag and sorry for the late reply, I was AFK for a while. Working on the update to the AUR package right now :) |
No problem. PS: The version tagged introduced the non backward compatible change but also a regression which was then fixed with 8b5ce57. |
Sure thing! This update is a bit complex (several user requests and changes to accommodate), should I open a new issue to discuss them with you if I have questions or do you have another preferred communication channel? |
Creating an issue labeled as question is fine. |
Not everything makes use of
morituri.common.directory.Directory
to decide where to read/write files, which leads to some files ending up in/.morituri/, and others in XDG directories (/.config/morituri, ~/.cache/morituri by default).Everything should either follow XDG or the old standard.
It would also make sense to only allow one location for the config file, especially if we are to rename it (i.e.
whipper.conf
rathermorituri.conf
). Right now both ~/.config/morituri/morituri.conf and ~/.moriturirc are allowed.The text was updated successfully, but these errors were encountered: