Rendering of README.md inline with inner tree view dirs

[andy at warmcat.com (Andy Green)]

On 06/12/2018 05:31 PM, John Keeping wrote:
> 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:

>> 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.

OK.  I added a new cgit-scope variable that appends filenames to be 
considered for showing in tree view, eg,

tree-readme=README.md

>> 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.

I'll leave this as a list for the first try.  If it looks excessive I'll 
reduce it to just the one.

>> 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.

OK.  Unfortunately I don't know python very well.  It looks like the 
markdown python library is able to be told to use extensions that are 
capable to do this

https://python-markdown.github.io/extensions/api/

from the md2html wrapper.  But I don't know enough python to do it.

It's a shame, because in-tree assets correctly follow the ref context 
being viewed, eg, if you look at a v2 branch you see v2 pngs, master you 
see master pngs etc.

I'll "solve" this part for now by changing the README to use external URLs.

> 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.)

Not sure what you are asking here... what's happening is the markup to 
be rendered has a relative URL in its un-rendered form.  The patches 
didn't generate it, it's a relative URL in the blob we pulled out from 
the repo to be rendered.

The only guy who knows how to understand there is a relative URL there 
is the renderer app, like md2html, not the patches that arrange for it 
to be called.

If you mean your series implies a /render/ or something, I don't know, I 
only use it from /tree/.

>> 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().

I audited all uses of it and fixed missing frees in ui-tree and ui-blame.

I'll check it over and post the series here shortly.

-Andy

> 
> John
>