[PATCH v4 00/16] Render READMEs inline in tree view

[Jason at zx2c4.com (Jason A. Donenfeld)]

Hey John,

Thanks tons for your input, as always.

On Wed, Jun 27, 2018 at 9:51 PM John Keeping <john at keeping.me.uk> wrote:
> - It is desirable to have the existing source view in addition to the
>   rendered content, preferably with syntax highlighting via the source
>   filter; for example Markdown, HTML or SVG can be sensibly viewed in
>   both ways

Got it, so this is the big kicker. Essentially we need a nice way for
the render filter to fallback to the source filter, as you said, with
then a `&source=1` override (and link) if a user explicitly wants the
source, in order to have both views. That seems like a good idea. This
would solve the issue with line numbers as well.

But how to implement this efficiently? With the lua filters, it really
doesn't matter, but with the exec filters, we perhaps incur the
penalty of exec'ing twice. Maybe that's acceptable, though. The other
approach would be for the source-filter to print its own line numbers,
and so the render filter itself could just fall back to calling
internally the source filter, and perhaps even indicating what it did
via the exit code, but I think I like this idea less. A third option
is to do this selection entirely within cgit via mimetype/fileext
selection, while I was initially hesitant about this, maybe this is an
okay approach after all.

Assuming we go with the first approach -- of the renderer fallback --
we would then have:

- render-filter (nee about-filter)
- source-filter

Then, if render-filter returns an exit code, cgit assumes we're in
source filter mode.

>
> - For some content types, the easiest way to render it is to simply
>   include the file with an iframe; this is the case for images and PDFs
>   at the moment
>
> Now, if the render filter can say "I don't support this" and can do so
> reasonably efficiently, we can use that to control whether render mode
> is enabled and whether we use the "fallback to mimetype" to include an
> iframe.  Or with the asset path extension to the filter arguments, we
> could have the filter generate the <iframe> element itself [2].

This should certainly be up to the actual render script. This means
we'll need to path sensible blob paths so that the render script can
correctly fill in <img src="..." or populate a pdf.js output
correctly.

> I agree with unifying the concepts, but might want to bikeshed the name
> since if we go for render mode on all files then it applies to a bit
> more than just readme files.

You're right about that. render-filter it is then.

> I think a distinction between repo-level readme and readme that can
> apply to each directory is useful.  We can probably also fall back to
> directory-level readme config using the default branch and root
> directory if the repo-level config is unset.
>
> I'm not sure there's much value in removing blob references from
> about-readme, although extending it to allow specifying a tree and then
> looking for the readme filename would be neat.

Yes, right. So, about-readme takes these values:

about-readme=/path/to/absolute/file/not/in/a/git/repo
about-readme=BRANCH:file/in/branch.md
about-readme=BRANCH:    # selects a "readme" file from BRANCH
about-readme=:file/in/default/branch/stuff.md   # selects stuff.md
from default branch
about-readme=:    # selects a "readme" file from the default branch

This winds up being pretty powerful and relatively simple to
implement. What is a "readme" file? Well, that's given by
readme-filename, as mentioned earlier.

Organizationally, since now we're going to be introducing rendering
into quite a few places, I think we should do this in ui-render.c.

Putting all of this together, we now have the following:

- We retain commit-filter, source-filter, and owner-filter as we have them now.
- We rename about-filter to render-filter.
- render-filter receives a fuller filename with useful path
information for printing out iframes, img tags, and so forth.
- We remove readme and instead introduce readme-filename instead.
- We introduce a global and a .repo level about-readme that takes the
above syntax.
- We introduce tree-readme=1/0.
- Viewing files in /tree defaults to render-filter. If that returns
error, it falls back to source-filter.
- Viewing files with /tree?source=1 goes straight to source-filter.
- When source-filter is used, cgit prints its usual line numbers.
- If source-filter fails, we print our nice hexdump or plaintext printer.
- When render-filter is used, cgit shows in the top bar a helpful link
to go to the source view.

Does that seem like a good synthesis of ideas? Or still more things to consider?

Jason