The logic isn't so easy to follow, especially since the loops in copy
and checksetup
imply that there are more lines, but really there will
be only two at most, the bookmark and backup folder. I'd suggest making
that more obvious and avoiding for line in file:
in case you only ever
want to have a fixed number of lines in that file.
The code also doesn't work with no "user.txt"
file present, so some
more checks and error handling are necessary to handle that case as
well.
Some of the paths and other constructs are duplicated, I'd always try to
minimise the amount of duplication, so with that in mind, consider more
constants and variables to extract common code.
- The
.seek(0)
in checksetup
isn't necessary with mode 'r'
; if you
open a file (for reading) you'll already be at the start; however, the
mode should probably be 'a+'
anyway to create the file if it doesn't
exist.
- If you don't like the escapes all the time, consider using the
r""
syntax instead.
- Use the
if __name__ == "__main__": ...
construct as is customary,
that way this file could be imported and used without running
checksetup
as is it would currently.
- Python typically uses lower case names with underscores between words,
c.f. PEP8.
- For the date formatting use the
strftime
method instead to get the
desired output directly.
- The
setup
method could use some more error checks, e.g. whether the
folders exist.
- The
"user.txt"
file is opened three times in the worst case; it
would also be better to pass the two values necessary for the script
in the script as function arguments instead of parsing the file so
often; I'll leave that be for the moment though. In general I'd say
that copy
should have the signature
copy(bookmark_directory, backup_directory)
and should be called with
the values from the file instead of parsing it.
After all that you can also look into command line arguments using
e.g. argparse
for
parsing options, that way users won't have to deal with pesky manual
input instead of reusing the shell completion (for specifying the
folders).
So, it should look more like this perhaps:
import shutil
import os
import datetime
SETUP_FILE = 'user.txt'
def setup():
print ('Enter your Chrome bookmark directory.')
print (r'Standard one is: C:\Users\<USER>\AppData\Local\Google\Chrome\User Data\Default')
bookmarkfolder = input('> ')
print ('Enter the folder you want to save the backup files in.')
print (r'For example: C:\Users\<USER>\Desktop\Chrome Bookmarks')
backupfolder = input('> ')
with open(SETUP_FILE, 'w') as txt:
txt.write(bookmarkfolder + '\n' + backupfolder + '\n')
copy()
def copy():
with open(SETUP_FILE, 'r') as txt:
user_list = [line.strip() for line in txt]
shutil.copy(user_list[0] + r'\Bookmarks', user_list[1])
new_name = datetime.date.today().strftime('Bookmarks%Y%m%d')
try:
os.rename('Bookmarks', new_name)
except WindowsError:
os.remove(new_name)
os.rename('Bookmarks', new_name)
def checksetup():
try:
with open(SETUP_FILE, 'r') as txt:
txt.seek(0)
line = txt.readline()
if '\\' in line:
copy()
else:
setup()
except FileNotFoundError:
setup()
if __name__ == "__main__":
checksetup()