4
\$\begingroup\$

I write a lot of TODO's but I never keep track of where they are.

This program will search through files (and recursively through folders if needed) and find any TODO comments and their line number.

The details will be printed to the screen and a file will be generated in the folder which is a csv of the files and TODO details.

You can choose different filetypes, comments and TODO phrases to suit , so it is not just for python.

GitHub

from os.path import exists
import sys
from glob import glob


class TodoLocater:
    def __init__(self, path, ext, td, comment, recursive=False):
        self.path = path
        self.ext = ext
        self.td = td
        self.comment = comment
        self.recursive = recursive
        self.todos = {}

    def get_files(self):
        try:
            g = glob(self.path + f"/**/*{self.ext}", recursive=self.recursive)
            if exists(self.path):
                for x in g:
                    print(f"Searching  ::  {x}")

                    result = self.find_todo(x)
                    if result:
                        self.todos[x] = result
            else:
                raise OSError("Path does not exist.")
        except Exception as e:
            print(e)

    def find_todo(self, f):
        temp_todos = []
        line_no = 0
        with open(f, "r") as input_:
            for line in input_:
                line_no += 1
                if self.comment in line and self.td in line:
                    temp_todos.append([f, f"Line - {line_no}  ::  {line.strip()}"])
        return temp_todos

    def show_todos(self):
        line = "-" * 100
        self.get_files()
        for k, v in self.todos.items():
            print(f"\n{line}\n\n{k}")
            for x in v:
                print(f">>>{x[1]}")
            self.save_csv(k, [v])

    def save_csv(self, fn, todos):
        import csv

        with open(fn + ".csv", "w", newline="") as csvfile:
            w = csv.writer(csvfile, delimiter=",", quoting=csv.QUOTE_MINIMAL)
            for row in todos:
                for r in row:
                    w.writerow(r)


if __name__ == "__main__":
    path = "."
    ext = "py"
    td = "TODO"
    comment = '#'

    find = TodoLocater(path, ext, td, comment, recursive=True)
    find.show_todos()
\$\endgroup\$
  • 1
    \$\begingroup\$ You should never catch a general exception. Catch what you expect and let the other stuff fail. It makes it easier for debugging. (except Exception as e) \$\endgroup\$ – Error - Syntactical Remorse Jul 16 at 15:57
  • 2
    \$\begingroup\$ It's great that you went out and solved a real problem you had. That's a fantastic skill. I would point out however that in this scenario this entire script can be accomplished with something like grep -rn '# *TODO' . (-r recursively searches the current dir and gives you file names, -n prints line numbers). The wrapping on SO ruins the command, note that there's a space between the # and the * (this means zero of more spaces between the # and TODO). \$\endgroup\$ – Bailey Parker Jul 16 at 19:10
  • \$\begingroup\$ @BaileyParker I am aware of grepping but I use this on windows..:) \$\endgroup\$ – johnashu Jul 17 at 6:27
  • \$\begingroup\$ Using the Pycharm IDE, you can search through a project's TODOs with Alt 6. \$\endgroup\$ – HoboProber Jul 17 at 10:07
  • \$\begingroup\$ I use VSC :P lol but thanks for the tip.. There probably is similar in VSC as well but this creates a csv and this means it will also end up in git so I have quick access anywhere.. \$\endgroup\$ – johnashu Jul 17 at 10:18
1
\$\begingroup\$

Comments regarding the structure of the program:

  • Method names. The name of a method should summarize what it does. However, some of your method names are misleading because they do things they should not do.

    • When I saw the name get_files(), I thought that this method just returns all the files in path, which is not true;
    • I would assume show_todos() just prints the todos, however, you also call save_csv() method inside it.
  • Using classes. I would not create a TodoLocator class. It could be just a method to which path, ext, etc. are passed as parameters. You shouldn't use class fields if you can use local variables and pass them between methods (Similarly to how you should avoid using global variables).

Having said that, I would structure the program as follows:

        def _get_files(path, ext) -> List[str]:
            # return a list of files 

        def _find_todos_in_file(file, todo_token, coment_start) -> List[Tuple[str, str]]:
            # return a list of todos in the file

        def find_todos(path, ext, todo_token = 'TODO', comment_start = '#') -> List[Tuple[str, str]]:
            files = _get_files(path, ext)
            return [todo for file in files for todo in _find_todos_in_file(file, todo_token, coment_start)]

        def show_todos(todos: List[Tuple[str, str]]):
            # show todos

        def save_csv(todos: List[Tuple[str, str], file: str):
            # save todos to a csv file

        if __name__ == "__main__":
            todos = find_todos('.', ext='py')
            show_todos(todos)
            save_csv(todos, file)

Other comments:

  • use for line_no, line in enumerate(input_): instead of:

    line_no = 0
    with open(f, "r") as input_:
        for line in input_:
            line_no += 1
    
  • Regarding if self.comment in line and self.td in line: line: what if you have a line like this:

    TODOs.append(todo) # there are no todos in this line
    
  • I am not sure about whether you need a recursive param at all. When would you need to use recursive=False?

\$\endgroup\$
  • \$\begingroup\$ Method names have always been a weak point. You have a point about the classes. I work a lot with OOP in guis and DB's so its my first thought.. Class... I think methods can be good in this case.. Enumerate is one i should have seen! Kudos on the TODOs.append(todo). I put in a check if todo_index > comment_index: to combat this. \$\endgroup\$ – johnashu Jul 17 at 6:46
  • \$\begingroup\$ Recursive is simple.. If you have a large codebase but only want the TODOS from say 1 module, don't use recursion. If you want to assess ALL modules, or even an entire Hard Drive, then use recursion, but it will take longer.. \$\endgroup\$ – johnashu Jul 17 at 6:47

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.