Rendering of README.md inline with inner tree view dirs

[john at keeping.me.uk (John Keeping)]

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
> 
> https://libwebsockets.org/git/libwebsockets/tree/
> 
> and
> 
> https://libwebsockets.org/git/libwebsockets/tree/minimal-examples
> 
> 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:

	refs/heads/wiki:README.md

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
> 
> /git/libwebsockets/about/doc-assets/lws-overview.png
> 
> 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 
> working.
> 
> I can have the direct content from cgit generally, but either the markup 
> needs fixing up to
> 
> /git/libwebsockets/plain/doc-assets/lws-overview.png
> 
> 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
print_object().


John