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.

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

Conversation

craig65535
Copy link
Contributor

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.

@craig65535 craig65535 changed the title Fix bad_dir check, and make warning louder installinstallmacos.py: Fix bad_dir check, and make warning louder Apr 3, 2021
@craig65535
Copy link
Contributor Author

@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.

@craig65535
Copy link
Contributor Author

@gregneagle Should I close this and merge to a different fork? Please respond

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

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.

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

@craig65535
Copy link
Contributor Author

@gregneagle All comments addressed for this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants