Follow XDG base directory specification#104
Conversation
|
There's slightly unexpected behavior still, in that, if a user sets |
f8fa495 to
5029760
Compare
|
Hey, thanks for looking into this. :) For your question, well it would be best to honor the -c option, but it is acceptable like this I think. I agree with the changes here, but I thought about it a bit, and maybe we should also make use of the XDG_CACHE_HOME env var to put the cache used by the browser - which can be quite huge. Though this will be a more complex change I think, so maybe we can start with this. Do you want me to merge the patch as it is now? For the tests, have you followed https://github.com/parkouss/webmacs#running-tests? if yes, maybe you could tell me what's wrong, it is possible that the documentation here is incomplete. |
|
@parkouss I haven't quite grokked the whole project yet, so I wasn't sure how to move around the browser cache - definitely agree though, it's very handy to just go Maybe when I get a bit more time, might be a few weeks. It's up to you to tell if this is up to par as-is :) I have a fairly exotic setup and herbstluftwm didn't seem to like it much. I frankly gave up pretty quickly, I'll put a bit more effort in next time and document anything I need to do to get it to work over here. |
5029760 to
348408e
Compare
|
I've updated the patch :) It now also puts The default I don't think we need a separate deprecation warning for the cache, since it's fundamentally temporary data, so we should be free to clear it. |
f461474 to
f0c962e
Compare
To become a better citizen, we now follow the XDG base directory specifications - this makes it easier to configure where we dump data, and helps avoid cluttering users' home directories. We still read ~/.webmacs, but we give a deprecation warning - we intend to ignore this directory in the future.
parkouss
left a comment
There was a problem hiding this comment.
This is great work. Thanks; I started to test this and it seems to work well.
I have two minor comments below, could you address them please if you agree? If possible at the least, I would like the adblock dir to go in cache if not too hard.
If you agree, please add new commits so it will be easier for me to review.
| if os.path.isdir(deprecated): | ||
| # Since logging has not been setup yet, use manual print | ||
| print("Warning: '{}' as a data directory has been deprecated. " | ||
| "Please move your profile and adblock directories to '{}'." |
There was a problem hiding this comment.
adblock directory is only used for cache, it should go in the xdg_cache_home too.
There was a problem hiding this comment.
Also, the current profile dir unfortunately contain a "cache" subdirectory which should not be copied in new data dir. Maybe you could rephrase to ask for moving profile/default instead of just cache?
There was a problem hiding this comment.
also, I started to use the warning module in some other places, such as https://github.com/parkouss/webmacs/blob/master/webmacs/keymaps/__init__.py#L545. Do you mind giving it a try instead of raw prints?
There was a problem hiding this comment.
Yup, I'll get these things fixed. Sorry, I didn't realize the warnings module even existed!
There was a problem hiding this comment.
Ah, no problem at all! It is not really a blocking issue but it would be more consistent.
I'll wait for your updates then. :)
Fixes #95.
For now, this implements the second suggested strategy (continue supporting the old path with a deprecation warning, but prefer the new path), and splits the configuration and other data directories between
$XDG_DATA_HOMEand$XDG_CONFIG_HOME.Furthermore, this branch makes the configuration directory configurable with
-c. The profile/log/adblock directory is not configurable via CLI, this is intentional since IME other applications don't allow this either, and it seems a little pointless - the user should never need to touch these files anyway, and can always just use$XDG_DATA_HOME.I also don't really see the point to adding a
webmacs://configpage just to display this directory. Perhaps if we allowed in-browser editing of the init module, but that's a bit beyond the scope of this patch.I've done some smoketesting (and am using the branch as we speak!), but I can't get the tests to run :(