Review: Needs Fixing
Sorry, a few nit picks...
Diff comments:
> === modified file 'openlp/core/app.py'
> --- openlp/core/app.py 2019-03-28 21:03:32 +0000
> +++ openlp/core/app.py 2019-04-30 19:47:11 +0000
> @@ -301,6 +302,8 @@
> help='Set logging to LEVEL level. Valid values are
> "debug", "info", "warning".')
> parser.add_argument('-p', '--portable', dest='portable',
> action='store_true',
> help='Specify if this should be run as a portable
> app, ')
> + parser.add_argument('-pp', '--portable-path', dest='portablepath',
> default=None,
> + help='Specify the path of the portable data,
> defaults to "<AppDir>/../../".')
we have *some* window users can you make this:
'Specify the path of the portable data, defaults to
"{dir_name}".'.format(dir_name=os.path.join('<AppDir>', '..', '..'))
> parser.add_argument('-w', '--no-web-server', dest='no_web_server',
> action='store_true',
> help='Turn off the Web and Socket Server ')
> parser.add_argument('rargs', nargs='?', default=[])
> @@ -356,7 +359,14 @@
> application.setApplicationName('OpenLPPortable')
> Settings.setDefaultFormat(Settings.IniFormat)
> # Get location OpenLPPortable.ini
> - portable_path = (AppLocation.get_directory(AppLocation.AppDir) /
> '..' / '..').resolve()
> + if args.portablepath:
> + if os.path.isabs(args.portablepath):
> + portable_path = str_to_path(args.portablepath).resolve()
str_to_path isn't required here. str_to_path('') == None, Path('') == working
directory.
The line above (if args.portablepath:) already ensures that you're not passing
an empty str.
Make it:
+ portable_path = Path(args.portablepath)
> + else:
> + portable_path =
> (AppLocation.get_directory(AppLocation.AppDir) / '..' /
> + str_to_path(args.portablepath)).resolve()
No need for str_to_path here either, args.portablepath is a string, using the
'/' operator way of joining paths can take os.pathlike or string objects.
Make it:
+ AppLocation.get_directory(AppLocation.AppDir)
/ '..' / args.portablepath
> + else:
> + portable_path = (AppLocation.get_directory(AppLocation.AppDir) /
> '..' / '..').resolve()
As with the above two comments move resolve() down.
Make it:
+ portable_path = AppLocation.get_directory(AppLocation.AppDir) /
'..' / '..'
Finally, just the tidy the above up move each ".resolve()" down here:
+ portable_path = portable_path.resolve()
> data_path = portable_path / 'Data'
> set_up_logging(portable_path / 'Other')
> log.info('Running portable')
--
https://code.launchpad.net/~tomasgroth/openlp/portable-path/+merge/366734
Your team OpenLP Core is subscribed to branch lp:openlp.
_______________________________________________
Mailing list: https://launchpad.net/~openlp-core
Post to : openlp-core@lists.launchpad.net
Unsubscribe : https://launchpad.net/~openlp-core
More help : https://help.launchpad.net/ListHelp
