Sort function declarations and definitions by order of use

[guijan]

This piece of style has been bugging me for a long time. I usually fix a small portion of it if I touch a small function. We should order functions by order of use. Start reading main() and look at every function call, stepping into all function calls, and reorder function declarations and definitions by order of use. I know the ordering is wrong in src/options.c, it's probably wrong in other places too, I haven't looked.

Perhaps a tool like cflow can automate figuring out the order? I never figured that tool out.

This is going to cause a lot of merge conflicts so either do it in small parts to avoid touching the entire source code at once or wait until there are few PRs.

[N-R-K]

I'd honestly avoid large scale cosmetic changes (unless there's some solid benefit to it). They make it harder to understand the history of a specific snippet via git blame.

Also if we make such change, then it has to be maintained in future too. Which is unnecessary extra (busy)work for contributors. Perhaps this is not a huge deal for scrot since there's barely any contributors, but it still creates a slightly higher barrier for both contributors and reviewers. And we should probably avoid doing that just for cosmetic reasons.

(Theoretically, tools like clang-format can solve this. But in practice, auto-formatters tend to mess up edge cases really badly when run against the entire code base).

[guijan]

I think the current state of the code is more important than the past state of the code. Scrot at some point had no style, lots of dead or useless code, trailing whitespace, lots of useless #ifdef, and one big spaghetti header with the majority of includes and declarations used across the source code (and then some small headers with a fraction of the declarations/includes used in their associated .c files). Fixing this is yet another step away from people rightfully cringing when they look at our source tree.

Besides, I'm sure someone will find a bug or some bad code that can be rewritten better while working on this.

[N-R-K]

Fixing this is yet another step away from people rightfully cringing when they look at our source tree.

For me at least, what's cringe worthy is the fact that we're still working around hardware defects from the 70s. Function declarations, at least for static/internal functions, are entirely unneeded from a technical perspective (indeed, most (all?) modern languages do away with them).

And even in C, it's still unnecessary to have separate declaration for internal function if you put main() at the bottom (and similar). And at least in my experience 95% of projects seem to do this.

Here's a practical and recent example of the downsides of having separate declaration: https://github.com/resurrecting-open-source-projects/scrot/pull/323/commits/6772c8d5fe10ce555438494f20100a473d17e4ed - in most projects I would've been done after writing the definition of miliToNanoSec(). But due to scrot's structure, I needed to also go add a declaration.

Well, I can just copy paste the declaration from the definition and add a semicolon, right? Nop, I still now need to manually go clear the argument names because for some reason scrot's codebase doesn't have argument names (which can provide valuable information to the reader) in declaration.

It's an entire class of friction that we're still emulating even though we're no longer limited by the same hardware limitations of the 1970s. So if anything, if we're going to touch decel/def order, we should try to remove these frictions, not add more.

I.e reorder the definitions so that the static decels can be just removed. And possibly add names into the headers so that a reader can learn more about a function from just the decel, e.g void selectionEdgeMotionDraw(int, int, int, int); vs void selectionEdgeMotionDraw(int x0, int y0, int x1, int y1); rather than having to go hunt down the definition as well.

[guijan]

Declarations at the top are a nice summary of what's in the file, specially if they're ordered properly because that also gives an idea of the call graph. C is that way because of single-pass compilers, but that doesn't mean it's a curse. I also don't want to read the block of code inside a function top to bottom, but the function definitions themselves bottom to top, that's odd.

[N-R-K]

I also don't want to read the function definitions themselves bottom to top

The problem here is that, unlike written language, code isn't something that's strictly linear (top to bottom). You can have functions which are neither at the top nor bottom, but in a complete different file.

Which is why dev tooling (even back in the 80s and 90s) has gravitated towards being able to "jump to definition" with such feature either builtin in the editor or via external tools (ctags, cscope, LSP etc). This way, if you're reading a function and want to see the definition of something that's being called/referenced, you just "jump" to it. It doesn't matter whether it's top, bottom or a completely different file.

---

Sorry to say, but I really don't think this is a worthwhile goal to pursue.

[N-R-K]

Declarations at the top are a nice summary of what's in the file

Most code editors support code folding. E.g with vim foldmethod=syntax foldnestmax=1:

You can easily unfold the body, instead of having to jump from declaration to definition.

For more low-tech solution, you can also just grep the functions out (which is much easier when using a K&R style with the return type in it's own line).

Neither requires having to manually manage duplicated set of declaration and definition.