Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

installinstallmacos.py: Fix bad_dir check, and make warning louder #87

Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
072b65b
fix bad_dir check, and make warning louder
craig65535 Apr 3, 2021
217e987
remove debug print
craig65535 Apr 3, 2021
aeaa732
make more readable
craig65535 Apr 3, 2021
55779df
use os.path.join
craig65535 May 24, 2021
90dd3b9
Fix some pylint warnings
craig65535 May 24, 2021
4f32302
Address comments
craig65535 May 25, 2021
9990295
use startswith instead of in
craig65535 May 25, 2021
ea4ae17
revert to avoid future conflict
craig65535 May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions installinstallmacos.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -531,15 +531,17 @@ def main():
sys.exit('This command requires root (to install packages), so please '
'run again with sudo or as root.')

home_dir = os.path.expanduser("~")
current_dir = os.getcwd()
if os.path.expanduser("~") in current_dir:
bad_dirs = ['Documents', 'Desktop', 'Downloads', 'Library']
for bad_dir in bad_dirs:
if bad_dir in os.path.split(current_dir):
print('Running this script from %s may not work as expected. '
'If this does not run as expected, please run again from '
'somewhere else, such as /Users/Shared.'
% 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
Copy link
Contributor

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()

if bad_dir in current_dir:
print('*********************************************************', file=sys.stderr)
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)
Copy link
Contributor

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.

Copy link
Contributor Author

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?

Copy link
Contributor

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.

Copy link
Contributor

@gregneagle gregneagle May 24, 2021 •
edited
Loading

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.

Copy link
Contributor Author

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.


if args.catalogurl:
su_catalog_url = args.catalogurl
Expand Down