From mboxrd@z Thu Jan 1 00:00:00 1970 From: andy at warmcat.com (Andy Green) Date: Tue, 12 Jun 2018 13:53:27 +0800 Subject: Rendering of README.md inline with inner tree view dirs In-Reply-To: <20180611153858.GJ1922@john.keeping.me.uk> References: <20180611093152.4821159f@leda> <20180611095343.2865c71d@leda> <20180611153858.GJ1922@john.keeping.me.uk> Message-ID: 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? 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? 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/. 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? 5) I get some gcc 8.1 warnings, I made a couple of patches to get around them. In a couple of places, the code seemed legit really. gcc8.1: fix strncpy bounds warnings These warnings are coming on default Fedora 28 build and probably others using gcc 8.1 ../shared.c: In function ?expand_macro?: ../shared.c:483:3: warning: ?strncpy? specified bound depends on the length of the source argument [-Wstringop-overflow=] strncpy(name, value, len); ^~~~~~~~~~~~~~~~~~~~~~~~~ ../shared.c:480:9: note: length computed here len = strlen(value); ^~~~~~~~~~~~~ ../ui-shared.c: In function ?cgit_repobasename?: ../ui-shared.c:135:2: warning: ?strncpy? specified bound 1024 equals destination size [-Wstringop-truncation] strncpy(rvbuf, reponame, sizeof(rvbuf)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/shared.c b/shared.c index 21ac8f4..477db0a 100644 --- a/shared.c +++ b/shared.c @@ -480,7 +480,7 @@ static char *expand_macro(char *name, int maxlength) len = strlen(value); if (len > maxlength) len = maxlength; - strncpy(name, value, len); + memcpy(name, value, len); } return name + len; } diff --git a/ui-shared.c b/ui-shared.c index 9d8f66b..6656bd5 100644 --- a/ui-shared.c +++ b/ui-shared.c @@ -129,11 +129,12 @@ char *cgit_pageurl(const char *reponame, const char *pagename, const char *cgit_repobasename(const char *reponame) { /* I assume we don't need to store more than one repo basename */ - static char rvbuf[1024]; + static char rvbuf[1025]; int p; const char *rv; - strncpy(rvbuf, reponame, sizeof(rvbuf)); - if (rvbuf[sizeof(rvbuf)-1]) + + strncpy(rvbuf, reponame, sizeof(rvbuf) - 1); + if (rvbuf[sizeof(rvbuf) - 2]) die("cgit_repobasename: truncated repository name '%s'", reponame); p = strlen(rvbuf)-1; /* strip trailing slashes */ and also gcc8.1: fix strcat warning ../ui-ssdiff.c: In function ?replace_tabs?: ../ui-ssdiff.c:142:4: warning: ?strncat? output truncated copying between 1 and 8 bytes from a string of length 8 [-Wstringop-truncation] strncat(result, spaces, 8 - (strlen(result) % 8)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ui-ssdiff.c b/ui-ssdiff.c index 7f261ed..e520b95 100644 --- a/ui-ssdiff.c +++ b/ui-ssdiff.c @@ -118,7 +118,6 @@ static char *replace_tabs(char *line) int n_tabs = 0; int i; char *result; - char *spaces = " "; if (linelen == 0) { result = xmalloc(1); @@ -138,8 +137,17 @@ static char *replace_tabs(char *line) strcat(result, prev_buf); break; } else { + char *p; + int n; + strncat(result, prev_buf, cur_buf - prev_buf); - strncat(result, spaces, 8 - (strlen(result) % 8)); + + n = strlen(result); + p = result + n; + n = 8 - (n % 8); + while (n--) + *p++ = ' '; + *p = '\0'; } prev_buf = cur_buf + 1; } I should include these or the slightly dubious warnings are just regarded as an annoyance? Thanks, -Andy > > John >