From mboxrd@z Thu Jan 1 00:00:00 1970 From: john at keeping.me.uk (John Keeping) Date: Tue, 12 Jun 2018 10:31:45 +0100 Subject: Rendering of README.md inline with inner tree view dirs In-Reply-To: References: <20180611093152.4821159f@leda> <20180611095343.2865c71d@leda> <20180611153858.GJ1922@john.keeping.me.uk> Message-ID: <20180612093145.GL1922@john.keeping.me.uk> 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 > > 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. 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