From: Jon Seymour Date: Mon, 06 Jun 2005 01:09:42 GMT Subject: Re: [PATCH] Modify git-rev-list ... in merge order [ repost with bug fixes ] Message-ID: <2cfc4032050605180958fcf395@mail.gmail.com> In-Reply-To: On 6/6/05, Linus Torvalds wrote: > > > On Sun, 5 Jun 2005 jon@blackcubes.dyndns.org wrote: > > > > -static void show_commit(struct commit *commit) > > +static int show_commit(struct commit *commit) > > Ick. You've mixed "show_commit()" to be three totally independent things > - deciding whether to show at all > - showing the commit in traditional format > - showing the commit tree in the new "break" format > > I really hate functions that do totally unrelated things, and I'm so much > happier with the new show_commit() than the old "everything in one big > function" thing, that I'm unhappy about mixing the thing up again. > > Please leave show_commit() to just show the commit, and make the other > decisions be independent of that. My rationale was to re-use both the filtering logic currently in the show_commit_list while loop and the display logic, since I need both in order to maximise compatibility with the standard algorithm. However, I understand your concerns. My plan, therefore, is to split the filtering logic from show_commit_list's while loop into a separate function and create a third function which calls the filtering logic, then show_commit. I'll pass a pointer to the third function to sort_list_in_merge_order. I can leave the show_breaks functionality in show_commit (it is, after all, display functionality) or I can move it into the third function. Is that ok by you? jon.