Rendering of README.md inline with inner tree view dirs
john at keeping.me.uk
Tue Jun 12 11:31:45 CEST 2018
On Tue, Jun 12, 2018 at 01:53:27PM +0800, Andy Green wrote:
> On 06/11/2018 11:38 PM, John Keeping wrote:
> > On Mon, Jun 11, 2018 at 04:05:38PM +0800, Andy Green wrote:
> >> I think what github did comes into its own when you are in the mode of
> >> coming new to a project and trying to understand what you are even
> >> looking at from the project layout in the filespace. So they may well
> >> be clicking around in the tree view, it's perfect if any project
> >> documentation in that dir just magically appears after the already
> >> expected navigation context explaining it.
> >> Basically any available docs follow along contextually.
> >> I think it's desirable under all circumstances, since it's only coming
> >> if someone bothered to put relevant docs right there... if no way to do
> >> it right now and no better ideas, I will try to understand how cgit
> >> works for this tomorrow and see if any ideas.
> > I think this is a potentially useful feature; long before GitHub existed
> > web interfaces to file servers have included README content with the
> > directory listing.
> Yes indeed.
> > Given the render mode patches you listed above, I think it should be
> > reasonably straightforward to add this feature in ui-tree.c with the
> > following changes:
> > - Add fields to walk_tree_context to remember the filename and object_id
> > of a target README file (this needs some configuration to answer the
> > question: what is a README file?) and populate these during the normal
> > tree walk (probably in ls_item(), being careful to only accept blobs)
> > - In ls_tail(), if the walk_tree_context has valid values for those
> > fields, then read the blob content from the object_id and call
> > render_buffer()
> > This will re-use the existing render filter for "inline" README content,
> > which I think is a good thing. I think the filenames for READMEs will
> > have to be a new configuration (the existing "readme" configuration
> > takes a blob ref whereas this option wants a simple filename).
> Thanks for the suggestions, and the patches that are doing most of the
> work here.
> I got quite far with it, you can see
> are basically there. If it's helpful to post a WIP series happy to do it.
> 1) I did not attempt to have it learn about suitable filenames from the
> config yet, I just have ls_item() looking for "README.md". I saw
> there's already a way (readme=) for the config to list them for the
> about page... basically now tree view becomes a superset of the
> operation of the about page; the about page content appears in tree view.
> So do you have any thoughts about just re-using that list?
I think that list is refs to blobs, so you can specify something like:
but here we need base filenames to look for in the current directory, so
it will have to be a new config variable.
> 2) In the current patches, I allowed for ls_item to discover a
> linked-list of files and render them later one after the other. Eg, a
> dir of READMEs would render them like that. It's welcome or preferable
> to just restrict it to one?
My choice would be to take the first matching file, but I don't have a
strong opinion on what is the right behaviour.
> 3) You can see on the top level of the tree, the README.md references
> <img alt="lws-overview" src="./doc-assets/lws-overview.png">
> This url format works in github. In the cgit About view, this resolves to
> which also serves the right mimetype and content. So that kind of URL
> format is useful. But when we render the same markup and relative path
> via /tree/, it tries to show an html page containing the content.
> That's why the picture is missing in the /tree/ view... other pictures
> in that markup are coming with absolute URLs outside of cgit and are
> I can have the direct content from cgit generally, but either the markup
> needs fixing up to
> or /tree/ needs to learn to do what /about/ does.
> I'm wondering whether mmd2html might grow an environment var to know the
> base part for URLS that want to direct-render from cgit. Or if better
> to follow what /about/ did in /tree/.
Making tree do this will break the normal use of tree unless we add some
extra query parameter or path element. Given that, I think teaching the
renderer to use a path to /about/ is the right thing to do.
Does that affect the render mode patches as well? (This is a genuine
question, it's been a while since I wrote that code and I don't remember
testing files with embedded assets.)
> 4) Looking at read_sha1_file() in print_object(), I can't immediately
> see who frees that. Running cgit from the commandline, with valgrind,
> he seems to leak some things. Since it's a cgi, the policy is we don't
> worry too much about that, or I just miss the idea about where it's freed?
We don't generally worry about it too much, although it looks like
several other callers of read_sha1_file() do free the buffer. Since
it's easy to fit in here, it's probably worth freeing it at the end of
More information about the CGit