HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythongitMinor

Symlinking git-managed dotfiles into a home directory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
manageddirectorysymlinkingintogithomedotfiles

Problem

As a learning exercise, I decided to write a script for helping me maintain my Unix configuration/dot-files under a git repository. I check out a copy of the repository in my home directory called .dotfiles, and, inside that, I have the Python script called linkify, which deals with creating links from my home directory to the real files inside the .dotfiles directory.

For your reference, the respective code is in my GitHub repository.

```
#!/usr/bin/env python

import errno
import os

to_keep_private = [
'cifs-credentials',
'config',
'fetchmailrc',
'gitconfig',
'gnupg',
'mpoprc',
'msmtprc',
'mutt',
'netrc',
'offlineimaprc',
'pim',
'purple',
'quodlibet',
'ssh',
]

to_ignore = [
'.',
'.git',
'.gitignore',
'linkify',
]

def make_private(files=[]):
if len(files) == 0:
files = os.listdir('.')
else:
# Use the list of files passed by the user, if it is not empty
pass
for filename in files:
if os.path.isdir(filename):
os.chmod(filename, 0700)
else:
os.chmod(filename, 0600)

def linkify(files=[]):
if len(files) == 0:
files = os.listdir('.')
else:
# Use the list of files passed by the user, if it is not empty
pass

os.chdir(os.path.join(os.environ['HOME'], '.dotfiles'))
for source in files:
if source not in to_ignore:
target = os.path.join('..', '.' + source)
source = os.path.join('.dotfiles', source)
try:
os.unlink(target)
except OSError, e:
if e.errno == errno.ENOENT:
pass
elif e.errno == errno.EISDIR:
print("%s is a directory. Ignoring." % target)
else:
print errno.errorcode[e.errno]
finally:
os.symlink(source, target)

def add_hook():
f = open('.git/hooks/post-commit', 'w'

Solution

Some guidelines to follow:

-
If possible, as it is in your case, work with absolute path.

You'll have at the beginning to do more or less something like:

dir = '/home/rik/'
files = [os.path.join(dir,file) for file in os.listdir(dir)]


But everything should be easier and straighforward later on.

-
You need to move the default behaviour - files = os.listdir('.') - one level up.

I can see one symptom and one reason for that:

  • You've wrote two times the exact same code.



  • The two different functions with that code should ignore what's your default directory, since they operate on list of files.



So, if the user does not provide his own custom list, you need to generate the list of files in . and pass it to both your functions. This also means that the functions should not have a default argument value.

-
If you can, use argparse instead of optparse.

I'm not sure from which version optparse was deprecated, but the main point is that if you have argpare in your standard library use that. You have only a couple of arguments so switching shouldn't take long.

-
Be consistent with your variable names.

def make_private(files=[]):
    # [...]
    for filename in files:
        # [...]

def linkify(files=[]):
    # [...]
    for source in files:
        # [...]


Why one time files is a list of filenames and another a list of sources?

If you strongly believe that one function works on filenames and the other on sources maybe you need to change the argument name, otherwise being consistent will help the readability of your code.

-
As Winston Ewert already told you, you should use capital letter for your constants names. Quoting from PEP8:


Constants are usually defined on a module level and written in all
capital letters with underscores separating words. Examples include
MAX_OVERFLOW and TOTAL.

Additional notes

Don't do this:

def function(lst=[]):
    # ...


Do this instead:

def function(lst=None):
    if lst is None:
        lst = []
    # ...


The reason for that is well explained here (or google it, because it's a very well known problem in which everyone stumbles sooner or later).

I'd say that by doing this you'll also remove the useless else, but as I explained it above you should remove the whole check and move it one level up against the user arguments.

Code Snippets

dir = '/home/rik/'
files = [os.path.join(dir,file) for file in os.listdir(dir)]
def make_private(files=[]):
    # [...]
    for filename in files:
        # [...]

def linkify(files=[]):
    # [...]
    for source in files:
        # [...]
def function(lst=[]):
    # ...
def function(lst=None):
    if lst is None:
        lst = []
    # ...

Context

StackExchange Code Review Q#9657, answer score: 5

Revisions (0)

No revisions yet.