Gotodo - list all TODOs in your code

I use inline TODOs a lot in my code and needed a way to print all TODOs easily, so I created gotodo.

It will print all TODOs withing your code and you’ll even be able to filter them by assignee :

Message: Damn this package seems useless
File: mypackage/main.go:3

Assignee: asticode
Message: This variable should be dropped
File: mypackage/main.go:6

Message: Damn this function should be rewritten
Or maybe it should be dropped as well
File: mypackage/main.go:9

All needed in Linux system :slight_smile:

$ grep -rn -A 3 'TODO'

1 Like

I would see some value if the extraction was made from the AST instead of string search. The way it is done now, it might return false positives if the searched pattern is found within a constant, for instance.

Also it is not reading TODOs from multi-line comment blocks (/* ... */).

With all that said, it also means that if these two problems are fixed, this would be a very handy tool.

@oleg578 unfortunately this also returns “TODO” in none comments, this doesn’t allow to fetch and filter by assignee and if the TODO is more than 4 lines it doesn’t return all the info.

And to top all of that, it doesn’t use GO so this is no fun :smiley:

@ccirello Thanks a bunch for your smart remarks and encouragements.

I’ve made the changes you proposed and the package is now extracting the TODOs from the AST instead of string searches. Therefore it can read TODOs from multi-line comment blocks.

I’ve also used filepath.Walk instead of rewritting my own function and the package now proposes a “-e” flag to exclude one or more paths from the process.

Hope other people will find this tool handy :slight_smile:

Cheers

This is just splash remark only, not a sarcasm :slight_smile:
Also linux have powerful system tools to filter TODO with comments also.

I think that TODO sections can be managed by IDE or code editor.

This just remark :slight_smile:
Good luck for You

@oleg578 Yeah no worries, no offense taken.

In fact I was using my IDE a lot to manage my TODOs before so I get your point. But it didn’t handle assignees correctly and above all multi line TODOs so that’s why I decided to write my own TODOs extracter and post it here so that it might help somebody else.

Cheers

Looks improved, still some work to do (no pun intended).

You can improve a lot the readability of your code by indenting the error or alternative flows, ie, handling more elegantly the else blocks.

I see some cases of string subslicing for doing matching. Except for the fact that you’re normalizing to lower case, perhaps you could use stdlib’s strings.HasPrefix()

Also, you could reorganize your code in a way to make this tool also a reusable package. One alternative is to have one package with the logic, another package with some CLI wrapping, and the package main with few lines.

https://npf.io/2016/10/reusable-commands/

@ccirello thanks again for the smart input

I’ve reorganized the if/else in the extract function so that it’s more readable. Is it what you had in mind?

I’ve also used strings.HasPrefix() as much as possible as it is the smart thing to do.

As for making the package reusable, I do believe a GO projects should have as few subpackages/folders as possible as it doesn’t seem to be the way to go with this language (cf. the stdlib). Therefore I’ve kept the root of the project as a package named “todo”, reusable by other projects, and have added a “cmd” package that contains the buildable part. I think that fits well.

Thanks again
Cheers

There are improvements. Before I keep going on and on about your code I suggest you visit CodeReviewComments · golang/go Wiki · GitHub which is a nice list of topics to pay attention on.

Regarding:
https://github.com/asticode/gotodo/blob/master/todo.go#L38-L46

whenever I see nested if-blocks, I wonder whether they are masking for some boolean math that should be there but it isn’t. The way this block is written, you could remove at least one level of depth.

Regarding:
https://github.com/asticode/gotodo/blob/master/todo.go#L59-L63

Ditto. With an additional aspect that you are overwriting the err passed in functions params with the one gotten by todos.extractFile. Again, if by this point you don’t want to act on a directory, you can just return nil (I am not sure whether returning nil is the correct thing here. Early on you returned filepath.SkipDir)

I haven’t executed the code yet, but I wonder what would happen if the comment looked like this:

/*
TODO: something*/

How would this if-block behave? https://github.com/asticode/gotodo/blob/master/todo.go#L86-L88 – I see you skip the head of the comment, but does not check the footer.

I do not know whether you read Nystrom’s blog post about variable naming. If you haven’t, it is a very nice read: Long Names Are Long – journal.stuffwithstuff.com

So I am noticing at least one unnecessarily long variable name: TODOFound (which found is not a todo?)

OK. Now… this is totally a matter of personal opinion, feel free to discard as you will, but I feel using regex to extract assignee is a bit overkill, and probably slow. In the end, you want a very simple thing: find the position of first opening and closing parentheses, and subslice it. It could be accomplished with less, I assume. Check:

And finally, another matter of personal opinion and taste, I believe you could put some effort on the comments, by removing the trivial ones (such: https://github.com/asticode/gotodo/blob/master/todo.go#L37 for example), and improving the ones that matter, like this: https://github.com/asticode/gotodo/blob/master/todo.go#L29

Rob Pike’s once wrote a series of notes on C programming, and dedicated a whole topic about comments. It is also a good read: Rob Pike: Notes on Programming in C

I would highlight:

I tend to err on the side of eliminating comments, for several reasons. […] But I do comment sometimes. Almost exclusively, I use them as an introduction to what follows. Examples: explaining the use of global variables and types (the one thing I always comment in large programs); as an introduction to an unusual or critical procedure; or to mark off sections of a large computation.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.