[WIP] Change default location for config files to platform defined config folder (Close #42) #43
Conversation
|
The PR itself is fair enough, but does indeed break a lot for existing, casual users (myself included). If I'd merge this, then it would not read my current settings or stats. I think you need an upgrade strategy in the PR, where you copy the files over. Also, make sure |
|
Yah, breaking changes should definitely be avoided. I think the best solution would be to readd support for the currently used config file paths in this PR and default to that if the Also, the OSX standardization would need to be added to this PR, as mentioned in #42. |
cjbassi
changed the title
cslarsen
left a comment
cslarsen
left a comment
There was a problem hiding this comment.
Sorry for the late reply. If you fix get_config_directory so that it works absolutely the same way if you already have your files in ~/, then I can pull the PR.
Also, is it really common to put non-config files in XDG_CONFIG_HOME as well (I'm thinking about the CSV file)?
| self.filename = os.path.expanduser("~/.wpmrc") | ||
| config_directory = get_config_directory() | ||
| self.filename = os.path.join(config_directory, "wpmrc") | ||
| os.makedirs(config_directory, exist_ok=True) |
There was a problem hiding this comment.
I don't like creating this directory.
I'd rather want get_config_directory to (1) check if you already have ~/.wpmrc and just use that if people already have it there. If not, check that XDG_CONFIG_HOME is set and points to an existing directory, then return that if so.
| xdg_config_home = os.getenv("XDG_CONFIG_HOME") | ||
| if xdg_config_home == "": | ||
| xdg_config_home = os.path.join(os.getenv("HOME"), ".config") | ||
| return os.path.join(xdg_config_home, "wpm") |
| help="Shows CPM instead of WPM in stats") | ||
|
|
||
| argp.add_argument("--stats-file", default="~/.wpm.csv", type=str, | ||
| argp.add_argument("--stats-file", default="~/.config/wpm/wpm.csv", type=str, |
There was a problem hiding this comment.
I'd rather get the default path here from get_config_directory
| GTAGS | ||
| stats | ||
| .mypy_cache/ | ||
| .vscode/ |
There was a problem hiding this comment.
Please don't add these here. You can add your local gitignore under .git/info/exclude instead.
| ---------------------- | ||
|
|
||
| wpm will save scores in a CSV file in `~/.wpm.csv`. This file can be loaded | ||
| wpm will save scores in a CSV file in `~/.config/wpm/wpm.csv`. This file can be loaded |
There was a problem hiding this comment.
I'd rather just use wpm.csv since this will (according to what I propose) be in different directories, dependent on what system and setup you have. Then add a short section where you describe how we decide where the file goes (simple: first time user and you have XDG_HOME, it goes there, otherwise your home directory).
2 participants
This change makes wpm load and store config files to
~/.config/wpmby default, specifically thewpm.csvandwpmrcfiles. It also removes the current functionality of loading config files from the home directory, but let me know if I should readd that functionality as a fallback if the~/.config/wpm/*files don't exist since this would be a breaking change otherwise.