2
\$\begingroup\$

I would love to hear some comments on this small script, which (I hope) reduces repetitive tasks such as page refreshing, compilation and so on to minimum.

# -*- coding: utf-8 -*-
# Author: Jacek 'Sabr' Karolak (j.karolak@sabr.pl)
# Watch [files] for change, and after every change run given [command]
#
#
# Example watchdog script: safari.py
#  #!/usr/bin/env python
#  # -*- coding: utf-8 -*-
#
#  from loop import Loop
#
#  if __name__ == "__main__":
#      Loop(["osascript", "-e", """tell application "Safari"
#              do JavaScript "window.location.reload()" in front document
#          end tell"""]).run()
#   EOF
#
# Make safari.py executable (chmox +x safari.py) or use it with python.
# python safari.py FILES_TO_WATCH
# Whenever FILES_TO_WATCH change, refresh Safari in background :-)
#

__all__ = ["Loop"]

import argparse, subprocess
from os import getcwd, listdir, stat
from os.path import exists
from time import sleep


class Loop(object):

    def __init__(self, cmd):
        self._command = cmd
        self._files_to_watch = {}

    def _watchdog(self):
        '''Check wheter any file in self._files_to_watch changed,
        if so fire self._command'''

        check_file = lambda f: stat(f).st_mtime
        files = self._files_to_watch

        any_file_changed = False

        while True:

            # Check each file for st_mtime change (modification)
            for f in files.keys():
                actual_mtime = check_file(f)
                if not files[f] == actual_mtime:
                    any_file_changed = f
                    files[f] = actual_mtime

            if any_file_changed:
                # run command
                print('File: \'{}\' changed since last check.'\
                        .format(any_file_changed))
                any_file_changed = False
                subprocess.call(self._command)

            # sleep before next check
            sleep(0.5)

    def _set_files_to_watch(self, files):
        '''Process args files wheter they exists and include current directory
        content if requested.'''
        if '.' in files:
            files.remove('.')
            # combine all other given files with current working directory
            # content, without dot files
            files += [f for f in listdir(getcwd())\
                    if not f.startswith('.')]

        # make f list unique
        files = set(files)

        # check rights (in order to perform system stat) and wheter they exist
        for f in files:
            if not exists(f):
                msg = 'file \'{}\' does not exists, or I don\'t\
                        have access rights.'.format(f)
                raise IOError(msg)

        # save files to watch in instance variable
        self._files_to_watch = dict.fromkeys(files)

        # set modification times
        for file_key in self._files_to_watch.keys():
            self._files_to_watch[file_key] = stat(file_key).st_mtime


    def run(self):
        '''Parses command line arguments, processes files list,
        and fires watchdog.'''
        parser = argparse.ArgumentParser(description='Checkes wheter given \
                files change, when they do, runs given command.')
        parser.add_argument('files', metavar='F', type=str, nargs='+',\
                help='list files to process, if you add . in list it will\
                watch also all non dot files in current dir.\
                If you want to have all non dot files and some dot ones use:\
                \n.file1 .file2 .file2 . .file3 it will combine specified dot\
                files and all others.')

        args = parser.parse_args()

        self._set_files_to_watch(args.files)

        print('Started watching...')
        self._watchdog()


if __name__ == '__main__':
    pass
\$\endgroup\$
4
\$\begingroup\$
class Loop(object):

Loop isn't a very descriptive name for this object. Something like WatchDog or FileWatcher would make more sense

    def __init__(self, cmd):
        self._command = cmd
        self._files_to_watch = {}

This is a mapping from filenames to modification times, but the name doesn't reflect that. I suggest calling _file_modification_times.

    def _watchdog(self):
        '''Check wheter any file in self._files_to_watch changed,
        if so fire self._command'''

        check_file = lambda f: stat(f).st_mtime

check_file isn't a great name, because it doesn't tell me what the function actually does, return the modification time. Since its only used once in this function its also not clear whether its helpful.

        files = self._files_to_watch

What does copy _files_to_watch into a local variable help?

        any_file_changed = False

Move this inside the while loop. You aren't using it to carry information across loop iterations, so limit the scope to being inside the loop.

        while True:

            # Check each file for st_mtime change (modification)
            for f in files.keys():
                actual_mtime = check_file(f)
                if not files[f] == actual_mtime:
                    any_file_changed = f
                    files[f] = actual_mtime


            if any_file_changed:

A simpler approach:

new_modification_times = dict( (filename, os.stat(filename).st_mtime) for filename in files)
if new_modication_times != files:
     self._file_modification_times = new_modification_times

Generally, if you find yourself wanting to set boolean flags, that's a sign you aren't doing things the pythonic way.

                # run command
                print('File: \'{}\' changed since last check.'\
                        .format(any_file_changed))
                any_file_changed = False
                subprocess.call(self._command)

            # sleep before next check
            sleep(0.5)

    def _set_files_to_watch(self, files):
        '''Process args files wheter they exists and include current directory
        content if requested.'''
        if '.' in files:
            files.remove('.')

This special interpretation of '.' is likely to end up being confusing.

            # combine all other given files with current working directory
            # content, without dot files
            files += [f for f in listdir(getcwd())\
                    if not f.startswith('.')]

        # make f list unique
        files = set(files)

Since you put them in a dictionary later, uniquifiying here is kinda pointless.

        # check rights (in order to perform system stat) and wheter they exist
        for f in files:
            if not exists(f):
                msg = 'file \'{}\' does not exists, or I don\'t\
                        have access rights.'.format(f)
                raise IOError(msg)

Rather then prechecking this, why don't you just let the os.stat() complain when it tries to access the files

        # save files to watch in instance variable
        self._files_to_watch = dict.fromkeys(files)

        # set modification times
        for file_key in self._files_to_watch.keys():
            self._files_to_watch[file_key] = stat(file_key).st_mtime

There's no benefit to prepopulating the dictionary here. Instead, you can create an empty dictionary and loop over files instead of .keys(). Better yet, you can use the dictionary trick I used above to fill it. You can put it into a function and call it in both places.

    def run(self):
        '''Parses command line arguments, processes files list,
        and fires watchdog.'''
        parser = argparse.ArgumentParser(description='Checkes wheter given \
                files change, when they do, runs given command.')
        parser.add_argument('files', metavar='F', type=str, nargs='+',\
                help='list files to process, if you add . in list it will\
                watch also all non dot files in current dir.\
                If you want to have all non dot files and some dot ones use:\
                \n.file1 .file2 .file2 . .file3 it will combine specified dot\
                files and all others.')

        args = parser.parse_args()

        self._set_files_to_watch(args.files)

        print('Started watching...')
        self._watchdog()

You should probably put command line parsing logic somewhere besides this class. It makes it harder to reuse this class if it mixes command line parsing with actual logic. I'd parse a command line in a seperate main function and then have it call this.

if __name__ == '__main__':
    pass

Why?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.