Re: [PATCH] Modify git-rev-list ... in merge order [ repost with bug fixes ]

[Jon Seymour <jon.seymour@gmail.com>]

On 6/6/05, Linus Torvalds <torvalds@osdl.org> 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.