Add command-line flags, use stderr, other improvements #4
Conversation
echarlie
commented
Oct 19, 2017
echarlie
commented
- Makes pushbullet optional
- Adds libnotify support
- Adds oneshot mode
- Allow non-default config location
- Errors all printed to STDERR on *nix (no idea if this works on windows; don't really care)
Improvements: - There is now a flag (-p or --pushbullet) to send notifications via pushbullet. Otherwise, notifications are just printed to stdout. - Errors are now printed to stderr - Add oneshot mode (-o or --oneshot) to exit after checking once - Allow user to specify config (-f or --config), if they don't like hard-coded values
- new option for desktop notifications via libnotify: -d or --desktop - move requirements for pushbullet and libnotify alerts into optional.txt
| semester = str(config['year'])+terms.get(config['term']) | ||
|
|
||
| last_open_courses = list() | ||
|
|
||
| while True: | ||
| print("Starting check... ", datetime.now().strftime('%Y-%m-%d %H:%M:%S')) | ||
| print("Starting check... ", datetime.now().strftime('%Y-%m-%d %H:%M:%S'), file=sys.stderr) |
There was a problem hiding this comment.
Probably should print to stdout instead since it's not an error. If this is so it prints out right away, the stdout buffer can be flushed with sys.stdout.flush()
There was a problem hiding this comment.
I was thinking since it is largely diagnostic verbosity rather than the actual results that are wanted, stderr makes more sense; makes it easier to parse the actual results in a pipeline.
app.py
Outdated
| exit(1) | ||
| elif 'pb_token' not in config: | ||
| print('Config must contain your Pushbullet Access Token!') | ||
| elif use_pushbullet == True and 'pb_token' not in config: |
There was a problem hiding this comment.
This can just be elif use_pushbullet and 'pb_token' not in config since use_pushbullet is already a boolean. There are also a few other places below with the same thing when checking for pushbullet and libnotify.
app.py
Outdated
| exit(1) | ||
|
|
||
| pb = Pushbullet(config['pb_token']) | ||
| if use_pushbullet is True: |
There was a problem hiding this comment.
Same as above. if use_pushbullet:
app.py
Outdated
| print(err) | ||
| exit(1) | ||
|
|
||
| config_path = None |
There was a problem hiding this comment.
Initializing config_path is unnecessary since it will always be initialized in the if/else
app.py
Outdated
|
|
||
| print(message) | ||
| if oneshot == True: |
There was a problem hiding this comment.
Same things with the above if statements comparing to True
| @@ -17,15 +17,23 @@ | |||
| help="""The crn or course subject and number. | |||
| If using the course subject and number, they must be enclosed in quotation marks. | |||
| For instance: \"MATH 2214\" """) | |||
| parser.add_argument('-p', '--pushbullet', action='store_true', help='Send notification with pushbullet') | |||
| parser.add_argument('-d', '--desktop', action='store_true', help='Send alert via libnotify') | |||
| parser.add_argument('-f', '--config', help='Specify alternative config file') | |||
There was a problem hiding this comment.
Could this be -c so it's consistent with the others?
There was a problem hiding this comment.
Easy enough to do; -f is just slightly more consistent with short flags for other tools I've used; I think I had intended to use -c for --check, to check the config file for errors, and never got to implementing it
In response to review from amhokies
