On Mon, Feb 19, 2024 at 05:22:44PM -0800, Junio C Hamano wrote: > > This way it can be used outside of builtin/log.c. > > The next commit will make builtin/notes.c use it. > > The resulting file is only about a small part of the implementation > detail of "show", and has very little to do with "log". > > "show.c" that happens to house show_blob_object(), a "canonical" way > to display a blob object, with an anticipation that somebody may > want to expand it in the future to house the "canonical" way to > display a tag, a tree or a commit object, would be OK, though. I'm not sure this function (or its siblings in builtin/log.c) counts as "canonical", since it is deeply connected to "struct rev_info". So it is appropriate for log, show, etc, but not for other commands. It felt a little funny to me to make a new file just for this one function. The related functions are all in log-tree.c. And as a bonus, the name is _already_ horribly confusing because it says "tree" but the functions are really about showing commits. ;) All that said, I'm not sure based on our previous discussion why we can't just call stream_blob_to_fd(). Looking at show_blob_object(), most of the logic is about recording the tree-context of the given name and using it for textconv. But since we know we are feeding a bare oid, that would never kick in. So I don't know if there's any value in sharing this function more widely in the first place. -Peff
