Re: [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI

[Andy Green]

On 06/28/2018 07:30 AM, Jason A. Donenfeld wrote:

On Thu, Jun 28, 2018 at 1:24 AM Andy Green  wrote:

Can you help me understand what is "clearly superior" and "right way"
about the [...] menu in github?

Since you seem to be asking me to implement it?

What put me off it was there's only one useful option on it, and it
requires and extra click.


I think we should have all three of github's options -- copying the
range, copying a permalink to it, and a simple link to git blame. If
you think that's too arduous and only really care about copying, then


I do care about general alignment with github.  I want to replace github 
with this as far as it goes.



instead of [...] in the menu you can just put 📋 (or whatever other
glyph) as a small icon inside the range, as opposed to a large
slow-fading side link. Though, I still think the menu is better than
📋, because with 📋 it's still unclear what precisely is being copied.


That's true... I used it the other day and even I got the impression it 
would copy the highlit region.


OK I will look at the whole burger menu thing.

-Andy


Jason


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI

[Jason A. Donenfeld]

On Thu, Jun 28, 2018 at 1:24 AM Andy Green  wrote:
> Can you help me understand what is "clearly superior" and "right way"
> about the [...] menu in github?
>
> Since you seem to be asking me to implement it?
>
> What put me off it was there's only one useful option on it, and it
> requires and extra click.

I think we should have all three of github's options -- copying the
range, copying a permalink to it, and a simple link to git blame. If
you think that's too arduous and only really care about copying, then
instead of [...] in the menu you can just put 📋 (or whatever other
glyph) as a small icon inside the range, as opposed to a large
slow-fading side link. Though, I still think the menu is better than
📋, because with 📋 it's still unclear what precisely is being copied.

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI

[Andy Green]

On 06/28/2018 02:07 AM, Jason A. Donenfeld wrote:

Nack. If we're going to add something like this, the github GUI for it
is clearly superior and the right way of going about things, with the
elegant [...] menu.


"Nack"...

Can you help me understand what is "clearly superior" and "right way" 
about the [...] menu in github?


Since you seem to be asking me to implement it?

What put me off it was there's only one useful option on it, and it 
requires and extra click.


-Andy
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[Jason A. Donenfeld]

Hey John,

Thanks tons for your input, as always.

On Wed, Jun 27, 2018 at 9:51 PM John Keeping  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  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  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
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[Andy Green]

On 06/28/2018 06:46 AM, Jason A. Donenfeld wrote:

Hi Andy,

I'm happy to engage technically here in order to find the best way of
going about this. However,


Nobody is paying me to do it and I don't have an endless budget of time
to lavish on it (and it seems, neither do you...).
If cgit can't do what I need in a reasonable timescale, even with my
contribution, my choices are:
been shaped only by github.  It's not that hard to overstate its
importance but in a few ways, unless something in cgit is really better,
the way forward is to align, or align and improve... IMHO.


I don't see the relevance of whether or not you're paid and how much
time you have to implement things, or what predictions are about sink
or swim with regards to Github.


Mmm I could tell from your reply it's irrelevant to you.

That's why I pointed it out, because it's very relevant to me and will 
inform what I choose to do.



I think the general ideas you've presented in these patchsets are good
ones, and now it's time to collaborate on the best way to go about
doing it. Part of what we're encountering here is the fact that cgit
hasn't originally had a robust display/hosting intent; it was mostly
around showing the state of the git repository. Now we're trying to
grow beyond that. Hence, I'd like to organize this in a sensible way
so that such rendering capabilities are available at the heart of
cgit, rather than haphazardly added on with a smattering of


It's not that I don't understand the reasoning.  There is a lot of cruft 
because the project has been around a long while.  But it's not my cruft.



contradictory configuration switches. In spite of your pay grade, your
time budget, or your timescale, I'm going to make sure this gets done
in the right way.
I also run a FOSS project that has been around for years and have dealt 
with hundreds of contributions.


These features have been around for 10 years in github and still not in 
cgit, done right or wrong.  I think everyone will be better off if we 
can find a way.


-Andy


Jason


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[Andy Green]

On 06/28/2018 03:51 AM, John Keeping wrote:

Hi Jason,

On Wed, Jun 27, 2018 at 07:18:57PM +0200, Jason A. Donenfeld wrote:

With the current state of this series, cgit would have the following options:

- render.
- inline-readme
- render-filter


This one is only a concept, not a configuration value (just a note since
I couldn't remember introducing it!)


- about-filter
- commit-filter
- source-filter
- owner-filter
- readme=file
- readme=:file

Whoa nelly. Of these, about-filter, commit-filter, source-filter, and
owner-filter all have analogs in the repo.* namespace, which makes
sense; it seems this was omitted from the render-filter introduced by
this series. The thing that unifies about-filter, commit-filter,
source-filter, and owner-filter is that they all modify some aspect of
the rendered output, either via fork/exec or via lua.

In adding readme files under the tree view, the obvious observation is
that this is pretty much the same type of rendering that we're doing
in about-filter.

In adding rendering of arbitrary files in blob view, this is
essentially a fancy source view, with the one caveat of our
interesting handling of line numbers.

So, I'd propose the following re-organization (and after we nail it
down, we can bikeshed about compatibility with old configs, but for
now let's focus on ideal design):


I'll comment on the proposals inline below, but before doing that let me
try to remember why "render." exists in the first place (it's a
couple of years since I wrote that patch so I'm not entirely sure this
is the rationale I had at the time!)

Thinking though it now, I think about-filter and the render-filter
concept are equivalent.  And for the purpose of putting an inline
directory readme in the tree view, that may be sufficient.


That's also how it seems to be having meddled about in there a bit now. 
In fact just globally the whole "about" thing is a less flexible version 
of what this is trying to do.



However, my series was adding a new UI mode and there are a couple of
requirements there that I can't see a way to fix without render. or
something equivalent [1]:

- 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

- 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  element itself [2].


- We retain commit-filter, source-filter, and owner-filter as we have them now.
- We rename about-filter to readme-filter.


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.


Just ejecting about* and replace with render will do it and keep the 
number of implementations related to this at 1.



- We remove `readme` and instead introduce `readme-filename`, which
can be specified multiple times as is habit. This would simply take
the set of filenames considered to be readme files (readme.md,
readme.txt, etc). [Bikeshed discussion: case insensitive?]
- We introduce an options at the global level and at the .repo level
of `about-readme=/path/to/absolute/file` and `about-readme=:`
and `about-readme=:`. The first would replace our original usage of
`readme=/path/to=file`, and the second would replace the use of
`readme=branch:whatever`, specifying an explicit branch (like
cgit.git's wiki branch), and the third would indicate the default
branch.


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.


It doesn't sound too hard but when would I use it?  Is it a case where 
rendering a list of matches would solve it?


-Andy


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.


- We introduce an option at the global level and at the .repo level of
`tree-readme=1/0` to display (or not) the readme under each tree.


Agree.


- We do not introduce render-filter. We do not introduce render.;
such extension selection is successfully handled by the various
filters themselves already.


Agree on render-filter, but we need a better proposal for how to handle
the  case and the "render not supported" case if we want to do
those outside the filter.


[1] This could be the exit status 

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

[Jason A. Donenfeld]

Hi Andy,

I'm happy to engage technically here in order to find the best way of
going about this. However,

> Nobody is paying me to do it and I don't have an endless budget of time
> to lavish on it (and it seems, neither do you...).
> If cgit can't do what I need in a reasonable timescale, even with my
> contribution, my choices are:
> been shaped only by github.  It's not that hard to overstate its
> importance but in a few ways, unless something in cgit is really better,
> the way forward is to align, or align and improve... IMHO.

I don't see the relevance of whether or not you're paid and how much
time you have to implement things, or what predictions are about sink
or swim with regards to Github.

I think the general ideas you've presented in these patchsets are good
ones, and now it's time to collaborate on the best way to go about
doing it. Part of what we're encountering here is the fact that cgit
hasn't originally had a robust display/hosting intent; it was mostly
around showing the state of the git repository. Now we're trying to
grow beyond that. Hence, I'd like to organize this in a sensible way
so that such rendering capabilities are available at the heart of
cgit, rather than haphazardly added on with a smattering of
contradictory configuration switches. In spite of your pay grade, your
time budget, or your timescale, I'm going to make sure this gets done
in the right way.

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[Andy Green]

On 06/28/2018 01:18 AM, Jason A. Donenfeld wrote:

Hey Andy,

Thanks for this patchset. It looks like this is shaping up into a nice
direction. However, I'm a bit concerned about our nobs becoming
slightly overlapping and incoherent, and I think that with this
series, we should also unify how we handle rendering.


Well, I can understand that from your POV.

However I also have a POV... "This series" represents my attempt to 
align cgit to a piece of github that my users will definitely miss. 
Nobody is paying me to do it and I don't have an endless budget of time 
to lavish on it (and it seems, neither do you...).


If cgit can't do what I need in a reasonable timescale, even with my 
contribution, my choices are:


 - become a cgit developer until the happy day comes

 - maintaining my own OOT branch until it can natively do the same thing

 - use something else


With the current state of this series, cgit would have the following options:

- render.
- inline-readme
- render-filter
- about-filter
- commit-filter
- source-filter
- owner-filter
- readme=file
- readme=:file

Whoa nelly. Of these, about-filter, commit-filter, source-filter, and
owner-filter all have analogs in the repo.* namespace, which makes
sense; it seems this was omitted from the render-filter introduced by
this series. The thing that unifies about-filter, commit-filter,
source-filter, and owner-filter is that they all modify some aspect of
the rendered output, either via fork/exec or via lua.

In adding readme files under the tree view, the obvious observation is
that this is pretty much the same type of rendering that we're doing
in about-filter.


Yeah, about has lost all meaning if this series gets in.


In adding rendering of arbitrary files in blob view, this is
essentially a fancy source view, with the one caveat of our
interesting handling of line numbers.

So, I'd propose the following re-organization (and after we nail it
down, we can bikeshed about compatibility with old configs, but for
now let's focus on ideal design):

- We retain commit-filter, source-filter, and owner-filter as we have them now.
- We rename about-filter to readme-filter.
- We remove `readme` and instead introduce `readme-filename`, which
can be specified multiple times as is habit. This would simply take
the set of filenames considered to be readme files (readme.md,
readme.txt, etc). [Bikeshed discussion: case insensitive?]


... what's actually wrong with aligning with The Github Way?


- We introduce an options at the global level and at the .repo level
of `about-readme=/path/to/absolute/file` and `about-readme= > and 
`about-readme=:`. The first would replace our original usage of


How "about" just drop about-*, and the about tab, ui-about-* etc.

Set the readme names using both [repo.]about-readme= for compatibility 
and [repo.]readme=


Almost all projects have a README of some kind at the root of their 
source tree.  It was a standard layout for decades.  So this just 
replaces about.  And aligns with github.



`readme=/path/to=file`, and the second would replace the use of
`readme=branch:whatever`, specifying an explicit branch (like
cgit.git's wiki branch), and the third would indicate the default
branch.


... and don't do that, just show the inline README for the rev context.


- We introduce an option at the global level and at the .repo level of
`tree-readme=1/0` to display (or not) the readme under each tree.


... just always display any compatible readme a la github.


- We do not introduce render-filter. We do not introduce render.;
such extension selection is successfully handled by the various
filters themselves already.


That I don't know much about.  However, we can at least get rid of or 
repurpose about filter for this, if about if going away.


The copyright message says cgit has been around 22 years... since then 
programmers have been born and since the age of 12 their view of git has 
been shaped only by github.  It's not that hard to overstate its 
importance but in a few ways, unless something in cgit is really better, 
the way forward is to align, or align and improve... IMHO.


-Andy


John, Christian -- what are your thoughts on this?

Regards,
Jason


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 16/16] md2html: change css name to not conflict with highlight

[Andy Green]

On 06/28/2018 01:37 AM, Jason A. Donenfeld wrote:

Hi Andy,

This seems like an obvious thing to merge, but I'm actually not so
certain I understand its necessity. md2html uses the highlight class.
Our css uses the highlight class. You're saying this conflicts with
something? From where? Third-party CSS? If that's the case, and if


I'm saying blame renders wrongly on Fedora 28 for these reasons and the 
patch fixes it.



it's a serious problem to recon with, do we then want to just
namespace all of our classes as .cgit subclasses, or as
.cgit-highlight, .cgit-whatever prefixes?


I dunno what you want to do about it, but I wanted it to render 
correctly on Fedora 28, which this patch is enough to do.


-Andy

___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript

[Andy Green]

On 06/28/2018 02:02 AM, Jason A. Donenfeld wrote:

Hi Andy,

I'm super hesitant about the Pandora's box that introducing javascript
implies, but perhaps there's no use in fighting the future.


The "fighting the future" bus left the station ten years ago when github 
was founded.  Worrying about JS is just fighting the past.



A few notes:

- Your js needs the copyright line like the rest of the project.
- Rather than awkwardly namespacing global methods, wrap everything
inside a big anonymous function.


OK.


- 1.5s is way too long for an animation. In fact, I'm not sure what an
animation communicates at all, and perhaps we could get rid of it.


We can't apply the highlight until the page completes its load event, 
because the vertical layout is subject to be adjusted by image loads etc 
right up until then.  It looked nicer to me to have the highlight 
"become apparent" in the case the lines had already been visibly 
rendered without it.



- Setting colors from inside js is a big no-no. Instead, add and
remove classes from elements, and leave the styling to css.


OK.


Also, can this be done from pure css? For example, certainly
highlighting one line in pure css based on the #anchor is possible,
via css link selectors.


Maybe you are thinking something I'm missing, but there are three 
unrelated s there that happen to be rendered vertically aligned. 
They're not on a single  despite it may look like that.


-Andy


Jason


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v3 1/1] snapshot: support tar signature for compressed tar

[John Keeping]

On Wed, Jun 27, 2018 at 06:34:56PM +0200, Jason A. Donenfeld wrote:
> I've merged all the surrounding changes, but I'm not quite satisfied
> with the implementation of this one.
> 
> > +   for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix, ".tar") 
> > != 0; f_tar++)
> > +   /* nothing */ ;
> > +
> > +   } else if (starts_with(f->suffix, ".tar") && 
> > cgit_snapshot_get_sig(ref, f_tar)) {
> > +   strbuf_setlen(&filename, strlen(filename.buf) - 
> > strlen(f->suffix));
> > +   strbuf_addstr(&filename, ".tar.asc");
> > +   html(" (");
> > +   cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> > +  filename.buf);
> > +   html(")");
> 
> Can we, instead, _not_ special case .tar, but rather just allow for
> all signatures, if the note .asc exists? We don't want to serve
> arbitrary tarballs and archives, because this means load and bandwidth
> for the server that wasn't explicitly opted in by the admin, but all
> signatures are necessarily explicitly uploaded, so why restrict them
> from being downloaded?

I'm not quite sure what you're asking here, this is just printing the
signature link after the snapshow download link.

The idea here is that if you are downloading a .tar.gz then the
signature for the base .tar is better (it's easier to consistently
generate a .tar than it is a .tar.gz), so the admin will choose to
provide .tar.asc instead of .tar.gz.asc.

I would quite like to avoid special-casing .tar in the code like this
and instead allow a fallback option (or even bitmask) in the formats
table as a more generic implementation, but I don't think that's your
complaint here (I also don't think we'll ever add it for other formats,
so hardcoding .tar isn't too bad).


John
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: Fancier Source view [Was: Re: [PATCH v4 00/16] Render READMEs inline in tree view]

[John Keeping]

On Wed, Jun 27, 2018 at 07:26:13PM +0200, Jason A. Donenfeld wrote:
> Splitting out this issue into a different thread, because I think it's
> orthogonal to the other topic.
> 
> On Wed, Jun 27, 2018 at 7:18 PM Jason A. Donenfeld  wrote:
> > In adding rendering of arbitrary files in blob view, this is
> > essentially a fancy source view, with the one caveat of our
> > interesting handling of line numbers.
> 
> People click a .c file and they get pygments highlighting. That's
> good. People click a .pdf or a .md and they get either a hexdump or a
> highlighted .md source. Github has made different behavior more
> popular -- namely, rendering the .pdf and the .md. The obvious way to
> handle this is augmenting the source-view to spit something different
> out.
> 
> Alternatively, my previously proposed readme-filter could be renamed
> to render-filter, and we could do everything in one place with one
> filter, and decide exclusively on the basis of filetype.
> 
> Either way, we have this issue of line numbers. We have a few ways of
> handling this:
> 
> a. Not print them ever when a source-filter/render-filter is
> specified, leaving that up to the renderer to do.
> b. If the renderer doesn't want line numbers, it can return a special
> exit code (197, for example) to indicate that to cgit.
> c. If the renderer doesn't want line numbers, it can just omit some
> !important css to remove them at the end.
> d. We introduce a maze of filetype selection rendering logic options.
> e. Something else?
> 
> I'm most inclined to go with a or b, but neither seems entirely
> appealing. Thoughts?

d. is basically what we have with "render." in this series, but I
think e. might be better.  Specifically, I think we shouldn't conflate
source and rendered views because syntax highlighted Markdown, or SVG,
or HTML is still potentially useful.

If source and rendered views are distinct, then source view always has
line numbers (or it's a hex dump) and we just need the filter to return
a special exit status to mean "this file is unsupported", and we disable
the rendered view when that happens.

As an extension, we could disable the source view if rendering is
supported and the file looks like binary, so that the hex dump isn't
available when we have a better option.


John
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 11/16] md2html: add asset mapping

[John Keeping]

On Wed, Jun 27, 2018 at 07:32:56PM +0200, Jason A. Donenfeld wrote:
> On Wed, Jun 20, 2018 at 12:13 PM Andy Green  wrote:
> 
> > md2html  >
> > The trailing "/" is important.
> 
> Can we make it not important? That is, if the type is always
> explicitly a directory, treat it as such using the usual file name
> joining logic.

Seems sensible.  This was originally posted as a proof of concept, so
would benefit from a bit of tidying.

I'm not actually sure if it does the right think when cfg.virtual_root
is NULL so we may need something a bit more complicated than this (and I
can't see a way that doesn't require the filter to handle both types of
URL).

> Alternatively, and perhaps better, don't introduce a second argument.
> Just pass the full file path in the first argument, and have asset
> mapping always work it out in the right way.

It depends how we think about the first argument.  The URL of the
"current page" when we call the filter will be something like:

/repo/tree/subdir/README.md

but the asset path should point to:

/repo/plain/subdir/

We could just say it's the URL of the input file, which is actually
quite nice if we want the filter to be able to replace the content with
an iframe.

However, I think there's a big benefit in backwards compatibility if we
keep the first argument as a file name and don't change it to a URL.
But that doesn't stop us making the new argument into a URL to the file.


Regards,
John
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[John Keeping]

Hi Jason,

On Wed, Jun 27, 2018 at 07:18:57PM +0200, Jason A. Donenfeld wrote:
> With the current state of this series, cgit would have the following options:
> 
> - render.
> - inline-readme
> - render-filter

This one is only a concept, not a configuration value (just a note since
I couldn't remember introducing it!)

> - about-filter
> - commit-filter
> - source-filter
> - owner-filter
> - readme=file
> - readme=:file
> 
> Whoa nelly. Of these, about-filter, commit-filter, source-filter, and
> owner-filter all have analogs in the repo.* namespace, which makes
> sense; it seems this was omitted from the render-filter introduced by
> this series. The thing that unifies about-filter, commit-filter,
> source-filter, and owner-filter is that they all modify some aspect of
> the rendered output, either via fork/exec or via lua.
> 
> In adding readme files under the tree view, the obvious observation is
> that this is pretty much the same type of rendering that we're doing
> in about-filter.
> 
> In adding rendering of arbitrary files in blob view, this is
> essentially a fancy source view, with the one caveat of our
> interesting handling of line numbers.
> 
> So, I'd propose the following re-organization (and after we nail it
> down, we can bikeshed about compatibility with old configs, but for
> now let's focus on ideal design):

I'll comment on the proposals inline below, but before doing that let me
try to remember why "render." exists in the first place (it's a
couple of years since I wrote that patch so I'm not entirely sure this
is the rationale I had at the time!)

Thinking though it now, I think about-filter and the render-filter
concept are equivalent.  And for the purpose of putting an inline
directory readme in the tree view, that may be sufficient.

However, my series was adding a new UI mode and there are a couple of
requirements there that I can't see a way to fix without render. or
something equivalent [1]:

- 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

- 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  element itself [2].

> - We retain commit-filter, source-filter, and owner-filter as we have them 
> now.
> - We rename about-filter to readme-filter.

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.

> - We remove `readme` and instead introduce `readme-filename`, which
> can be specified multiple times as is habit. This would simply take
> the set of filenames considered to be readme files (readme.md,
> readme.txt, etc). [Bikeshed discussion: case insensitive?]
> - We introduce an options at the global level and at the .repo level
> of `about-readme=/path/to/absolute/file` and `about-readme=:`
> and `about-readme=:`. The first would replace our original usage of
> `readme=/path/to=file`, and the second would replace the use of
> `readme=branch:whatever`, specifying an explicit branch (like
> cgit.git's wiki branch), and the third would indicate the default
> branch.

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.

> - We introduce an option at the global level and at the .repo level of
> `tree-readme=1/0` to display (or not) the readme under each tree.

Agree.

> - We do not introduce render-filter. We do not introduce render.;
> such extension selection is successfully handled by the various
> filters themselves already.

Agree on render-filter, but we need a better proposal for how to handle
the  case and the "render not supported" case if we want to do
those outside the filter.


[1] This could be the exit status of the render filter, but I'm not sure
how well the "probe" mode will fit it
[2] We do load the content redundantly in this case and we'll have to
make sure that our SIGPIPE handling is correct if the filter exits
without reading all of its stdin, but that's probably acceptable


Regards,
John
___
CGit mailing list
CGit@lists.zx2c4.com
ht

Re: [PATCH v5 6/6] line-range-highlight: copy URL to clipboard UI

[Jason A. Donenfeld]

Nack. If we're going to add something like this, the github GUI for it
is clearly superior and the right way of going about things, with the
elegant [...] menu.

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v5 2/6] cgit.js: line range highlight: introduce javascript

[Jason A. Donenfeld]

Hi Andy,

I'm super hesitant about the Pandora's box that introducing javascript
implies, but perhaps there's no use in fighting the future.

A few notes:

- Your js needs the copyright line like the rest of the project.
- Rather than awkwardly namespacing global methods, wrap everything
inside a big anonymous function.
- 1.5s is way too long for an animation. In fact, I'm not sure what an
animation communicates at all, and perhaps we could get rid of it.
- Setting colors from inside js is a big no-no. Instead, add and
remove classes from elements, and leave the styling to css.

Also, can this be done from pure css? For example, certainly
highlighting one line in pure css based on the #anchor is possible,
via css link selectors.

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Jenkins build is back to normal : cgit-master #179 - origin/master - c4fbb99

[Pelagic Jenkins (Public)]

See 


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Jenkins build is back to normal : cgit-master-get-git #178 - origin/master - c4fbb99

[Pelagic Jenkins (Public)]

See 


___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 13/16] ui-shared: deduplicate some code in repolink

[Jason A. Donenfeld]

And, reverted.

The code is not the same; this broke tests. html_url_arg != html_url_path.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 16/16] md2html: change css name to not conflict with highlight

[Jason A. Donenfeld]

Hi Andy,

This seems like an obvious thing to merge, but I'm actually not so
certain I understand its necessity. md2html uses the highlight class.
Our css uses the highlight class. You're saying this conflicts with
something? From where? Third-party CSS? If that's the case, and if
it's a serious problem to recon with, do we then want to just
namespace all of our classes as .cgit subclasses, or as
.cgit-highlight, .cgit-whatever prefixes?

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 11/16] md2html: add asset mapping

[Jason A. Donenfeld]

On Wed, Jun 20, 2018 at 12:13 PM Andy Green  wrote:

> md2html 
> The trailing "/" is important.

Can we make it not important? That is, if the type is always
explicitly a directory, treat it as such using the usual file name
joining logic.

Alternatively, and perhaps better, don't introduce a second argument.
Just pass the full file path in the first argument, and have asset
mapping always work it out in the right way.

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 13/16] ui-shared: deduplicate some code in repolink

[Jason A. Donenfeld]

Merged, thanks.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 01/16] manpage: fix sorting order

[Jason A. Donenfeld]

Merged, thanks.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v4 02/16] Use string list strdup_strings for mimetypes

[Jason A. Donenfeld]

Thanks John, merged.
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Fancier Source view [Was: Re: [PATCH v4 00/16] Render READMEs inline in tree view]

[Jason A. Donenfeld]

Splitting out this issue into a different thread, because I think it's
orthogonal to the other topic.

On Wed, Jun 27, 2018 at 7:18 PM Jason A. Donenfeld  wrote:
> In adding rendering of arbitrary files in blob view, this is
> essentially a fancy source view, with the one caveat of our
> interesting handling of line numbers.

People click a .c file and they get pygments highlighting. That's
good. People click a .pdf or a .md and they get either a hexdump or a
highlighted .md source. Github has made different behavior more
popular -- namely, rendering the .pdf and the .md. The obvious way to
handle this is augmenting the source-view to spit something different
out.

Alternatively, my previously proposed readme-filter could be renamed
to render-filter, and we could do everything in one place with one
filter, and decide exclusively on the basis of filetype.

Either way, we have this issue of line numbers. We have a few ways of
handling this:

a. Not print them ever when a source-filter/render-filter is
specified, leaving that up to the renderer to do.
b. If the renderer doesn't want line numbers, it can return a special
exit code (197, for example) to indicate that to cgit.
c. If the renderer doesn't want line numbers, it can just omit some
!important css to remove them at the end.
d. We introduce a maze of filetype selection rendering logic options.
e. Something else?

I'm most inclined to go with a or b, but neither seems entirely
appealing. Thoughts?

Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

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

[Jason A. Donenfeld]

Hey Andy,

Thanks for this patchset. It looks like this is shaping up into a nice
direction. However, I'm a bit concerned about our nobs becoming
slightly overlapping and incoherent, and I think that with this
series, we should also unify how we handle rendering.

With the current state of this series, cgit would have the following options:

- render.
- inline-readme
- render-filter
- about-filter
- commit-filter
- source-filter
- owner-filter
- readme=file
- readme=:file

Whoa nelly. Of these, about-filter, commit-filter, source-filter, and
owner-filter all have analogs in the repo.* namespace, which makes
sense; it seems this was omitted from the render-filter introduced by
this series. The thing that unifies about-filter, commit-filter,
source-filter, and owner-filter is that they all modify some aspect of
the rendered output, either via fork/exec or via lua.

In adding readme files under the tree view, the obvious observation is
that this is pretty much the same type of rendering that we're doing
in about-filter.

In adding rendering of arbitrary files in blob view, this is
essentially a fancy source view, with the one caveat of our
interesting handling of line numbers.

So, I'd propose the following re-organization (and after we nail it
down, we can bikeshed about compatibility with old configs, but for
now let's focus on ideal design):

- We retain commit-filter, source-filter, and owner-filter as we have them now.
- We rename about-filter to readme-filter.
- We remove `readme` and instead introduce `readme-filename`, which
can be specified multiple times as is habit. This would simply take
the set of filenames considered to be readme files (readme.md,
readme.txt, etc). [Bikeshed discussion: case insensitive?]
- We introduce an options at the global level and at the .repo level
of `about-readme=/path/to/absolute/file` and `about-readme=:`
and `about-readme=:`. The first would replace our original usage of
`readme=/path/to=file`, and the second would replace the use of
`readme=branch:whatever`, specifying an explicit branch (like
cgit.git's wiki branch), and the third would indicate the default
branch.
- We introduce an option at the global level and at the .repo level of
`tree-readme=1/0` to display (or not) the readme under each tree.
- We do not introduce render-filter. We do not introduce render.;
such extension selection is successfully handled by the various
filters themselves already.

John, Christian -- what are your thoughts on this?

Regards,
Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit

Re: [PATCH v3 1/1] snapshot: support tar signature for compressed tar

[Jason A. Donenfeld]

Hey Christian,

I've merged all the surrounding changes, but I'm not quite satisfied
with the implementation of this one.

> +   for (f_tar = cgit_snapshot_formats; strcmp(f_tar->suffix, ".tar") != 
> 0; f_tar++)
> +   /* nothing */ ;
> +
> +   } else if (starts_with(f->suffix, ".tar") && 
> cgit_snapshot_get_sig(ref, f_tar)) {
> +   strbuf_setlen(&filename, strlen(filename.buf) - 
> strlen(f->suffix));
> +   strbuf_addstr(&filename, ".tar.asc");
> +   html(" (");
> +   cgit_snapshot_link("sig", NULL, NULL, NULL, NULL,
> +  filename.buf);
> +   html(")");

Can we, instead, _not_ special case .tar, but rather just allow for
all signatures, if the note .asc exists? We don't want to serve
arbitrary tarballs and archives, because this means load and bandwidth
for the server that wasn't explicitly opted in by the admin, but all
signatures are necessarily explicitly uploaded, so why restrict them
from being downloaded?

Regards,
Jason
___
CGit mailing list
CGit@lists.zx2c4.com
https://lists.zx2c4.com/mailman/listinfo/cgit