Skip to content
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

add xdg support #866

Merged
merged 10 commits into from
Jan 11, 2018
Merged

add xdg support #866

merged 10 commits into from
Jan 11, 2018

Conversation

legrostdg
Copy link

Should fix #771

For now I've just put the pronsole history file in user_data_dir/pronsole/history if ~/.pronsole-history does not exist. Let me know if you agree this, and I'll add user_config_dir support for pronsolerc.

Please note that win32 and macos are supported by appdirs.

@rockstorm101
Copy link
Collaborator

Hi @legrostdg, I'm sorry it took me this long to review this. By the way its written it should not break any current installation. I've tested on Debian, but I cannot speak for Windows and Mac OS.

Anyway, if you don't mind me asking, I've never used appdirs. According to the source, user_data_dir is defined as:

def user_data_dir(appname=None, appauthor=None, version=None, roaming=False)

Yet you invoke it as:

user_data_dir("Printrun", "pronsole")

Why the second argument? Why "pronsole"?

Thanks

@legrostdg
Copy link
Author

@rockstorm101 I made a typo, I wanted to use "pronsole" as appname, and thought appauthor was kind of necessary for Windows. According to the source, appauthor falls back to appname, so we can just set appname. I don't know if you prefer "pronsole", "printrun" or "Printrun" there.

@rockstorm101
Copy link
Collaborator

Hi @legrostdg, thanks for answering. I would go with "Printrun" as the appname but I'll leave that decision to the project owners. (CC @kliment @iXce)

@kliment
Copy link
Owner

kliment commented Jan 7, 2018 via email

@legrostdg
Copy link
Author

OK, I've updated the appname to "Printrun" and added support for supporting user_config_dir. I was not sure of what to do with the sys.frozen logic on Windows, so I just removed this.

@kliment
Copy link
Owner

kliment commented Jan 8, 2018

The sys.frozen logic needs to stay - otherwise people who used printrun with a packaged executable will lose their config when they switch to this version. It was originally introduced because files that start with a . confuse some versions of windows explorer

@legrostdg
Copy link
Author

@kliment I reintroduced the sys.frozen logic, can you have a look to this?

@legrostdg
Copy link
Author

I still need to put the ~bak file in user_cache_dir/Printrun/.

@legrostdg legrostdg changed the title WIP add xdg support add xdg support Jan 8, 2018
@legrostdg
Copy link
Author

done.

@rockstorm101
Copy link
Collaborator

rockstorm101 commented Jan 10, 2018 via email

@kliment
Copy link
Owner

kliment commented Jan 10, 2018

Looks good to me, @rockstorm101

@legrostdg
Copy link
Author

Thank you for the review! I hope to see 1.6.1 soon :)

@rockstorm101
Copy link
Collaborator

Merging. Thanks again.

@rockstorm101 rockstorm101 merged commit c5fccd6 into kliment:master Jan 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support XDG_CONFIG_HOME and XDG_DATA_HOME
3 participants