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
_______________________________________________
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit
