-
Notifications
You must be signed in to change notification settings - Fork 525
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
installinstallmacos.py: Fix bad_dir check, and make warning louder #87
base: main
Are you sure you want to change the base?
Conversation
@gregneagle Any comments on this one? I can revert the louder warning if you'd like, but I think it's beneficial to have a bad_dir check that works for subdirectories under Downloads/Documents/etc. |
@gregneagle Should I close this and merge to a different fork? Please respond |
installinstallmacos.py
Outdated
print('*** Running this script from %s may not work as expected.' % current_dir, file=sys.stderr) | ||
print('*** If this does not run as expected, please run again from', file=sys.stderr) | ||
print('*** somewhere else, such as /Users/Shared.', file=sys.stderr) | ||
print('*********************************************************', file=sys.stderr) |
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.
Looks to me like these lines are longer than 80 characters. Please use pylint and make sure the lines are within the current line-lengths.
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.
@gregneagle Did you want me to wrap at 80, or pylint's default of 100?
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.
Pretty sure the existing script wraps at 80.
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.
Why not use the previous code formatting?
print('*********************************************************\n'
'*** Running this script from %s may not work as expected.\n'
'*** If this does not run as expected, please run again from \n'
'*** somewhere else, such as /Users/Shared.\n'
'*********************************************************'
% current_dir, file=sys.stderr)
or similar.
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.
@gregneagle Updated as suggested.
Most of the script wraps at 80, although there is one 85-char line (line 460). I left it alone.
installinstallmacos.py
Outdated
% current_dir, file=sys.stderr) | ||
bad_subdirs = ['Documents', 'Desktop', 'Downloads', 'Library'] | ||
for bad_subdir in bad_subdirs: | ||
bad_dir = home_dir + os.path.sep + bad_subdir |
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.
Really should be using os.path.join()
@gregneagle All comments addressed for this one. |
When reporting #86 I noticed that installinstallmacos.py checks which directory it is being run from, but that check was not working for me. This changes the check so it works with both the "bad" directories like Downloads, and also subdirectories under those directories.