From mboxrd@z Thu Jan 1 00:00:00 1970 From: andy at warmcat.com (Andy Green) Date: Wed, 13 Jun 2018 09:47:38 +0800 Subject: Rendering of README.md inline with inner tree view dirs In-Reply-To: <20180612093145.GL1922@john.keeping.me.uk> References: <20180611093152.4821159f@leda> <20180611095343.2865c71d@leda> <20180611153858.GJ1922@john.keeping.me.uk> <20180612093145.GL1922@john.keeping.me.uk> Message-ID: <29df7d5c-2556-86e7-45bd-08e1e71a1e57@warmcat.com> 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 >> >> lws-overview >> >> 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 >