List for cgit developers and users
 help / color / mirror / Atom feed
* Commit _subject_ filter, everywhere
@ 2017-11-29 21:10 
  2017-12-02 11:31 ` john
  0 siblings, 1 reply; 8+ messages in thread
From:  @ 2017-11-29 21:10 UTC (permalink / raw)


Hi Jason, all,

I got this idea to generate a link to the CI server job in front of
each commit message subject. Think small green/red boxes that are
clickable. I figure that getting the build status directly in cgit is
much more efficient than navigating the CI server itself, which
typically requires many clicks to find the build that comes out of a
given commit.

I had high hopes for the the commit-filter= setting, but found that

* commit-filter= is only run when showing a full commit, not other
places. I.e. it is applied when showing the "commit" tab, but not on
"summary", "logs" or "refs".
* commit-filter= is run twice on a commit message. Once for the
subject and once for the body. I expected only once. (Is this a bug?)

Any thoughts on if/how this use case can be supported? A separate
filter? Or simply run commit-filter= everywhere (breaking change)?

Best regards,
Bj?rn Forsman (happy cgit user)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-11-29 21:10 Commit _subject_ filter, everywhere 
@ 2017-12-02 11:31 ` john
  2017-12-02 12:33   ` 
  0 siblings, 1 reply; 8+ messages in thread
From: john @ 2017-12-02 11:31 UTC (permalink / raw)


On Wed, Nov 29, 2017 at 10:10:00PM +0100, Bjjjrn Forsman wrote:
> I got this idea to generate a link to the CI server job in front of
> each commit message subject. Think small green/red boxes that are
> clickable. I figure that getting the build status directly in cgit is
> much more efficient than navigating the CI server itself, which
> typically requires many clicks to find the build that comes out of a
> given commit.
> 
> I had high hopes for the the commit-filter= setting, but found that
> 
> * commit-filter= is only run when showing a full commit, not other
> places. I.e. it is applied when showing the "commit" tab, but not on
> "summary", "logs" or "refs".
> * commit-filter= is run twice on a commit message. Once for the
> subject and once for the body. I expected only once. (Is this a bug?)
> 
> Any thoughts on if/how this use case can be supported? A separate
> filter? Or simply run commit-filter= everywhere (breaking change)?

I was going to suggest introducing a new subject-filter which is used to
filter the commit subject wherever it is displayed, but there is a
subtlety which makes this more complicated in the summary, log and refs
pages than it is in the commit page.

When we are showing a single-line summary of a commit, the subject is a
hyperlink so adding new content outside the existing subject is not
possible with the obvious filter implementation.

One option would be to push the <a></a> tag generation down into the
filter, but I'd rather avoid that if possible.

What do you think about adding a "filter" which is called before the
commit subject is printed and allows outputting additional HTML content?
("filter" is in quotes since there will not be any content copied
through the program.)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 11:31 ` john
@ 2017-12-02 12:33   ` 
  2017-12-02 12:49     ` john
  0 siblings, 1 reply; 8+ messages in thread
From:  @ 2017-12-02 12:33 UTC (permalink / raw)


Hi John,

On 2 December 2017 at 12:31, John Keeping <john at keeping.me.uk> wrote:
> I was going to suggest introducing a new subject-filter which is used to
> filter the commit subject wherever it is displayed, but there is a
> subtlety which makes this more complicated in the summary, log and refs
> pages than it is in the commit page.
>
> When we are showing a single-line summary of a commit, the subject is a
> hyperlink so adding new content outside the existing subject is not
> possible with the obvious filter implementation.
>
> One option would be to push the <a></a> tag generation down into the
> filter, but I'd rather avoid that if possible.

Aha, I see.

> What do you think about adding a "filter" which is called before the
> commit subject is printed and allows outputting additional HTML content?
> ("filter" is in quotes since there will not be any content copied
> through the program.)

Sounds good to me. It covers my use case.

Perhaps this new "filter" should not have filter in its name.
Suggestion: commit-prefix=.

Oh, and this new command must somehow receive the commit id from cgit.
New environment variable?

Best regards,
Bj?rn Forsman


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 12:33   ` 
@ 2017-12-02 12:49     ` john
  2017-12-02 14:44       ` 
  0 siblings, 1 reply; 8+ messages in thread
From: john @ 2017-12-02 12:49 UTC (permalink / raw)


On Sat, Dec 02, 2017 at 01:33:28PM +0100, Bjjjrn Forsman wrote:
> On 2 December 2017 at 12:31, John Keeping <john at keeping.me.uk> wrote:
> > I was going to suggest introducing a new subject-filter which is used to
> > filter the commit subject wherever it is displayed, but there is a
> > subtlety which makes this more complicated in the summary, log and refs
> > pages than it is in the commit page.
> >
> > When we are showing a single-line summary of a commit, the subject is a
> > hyperlink so adding new content outside the existing subject is not
> > possible with the obvious filter implementation.
> >
> > One option would be to push the <a></a> tag generation down into the
> > filter, but I'd rather avoid that if possible.
> 
> Aha, I see.

After I wrote that I realised that for the existing commit-filter we
escape the content before writing to the filter.  So it might be
reasonable to pass the whole <a></a> tag through the filter.

That would make it easy for the filter to put extra content either
before or after the commit subject.  It also makes the filter
integration much simpler, for example in ui-log.c:

-- >8 --
diff --git a/ui-log.c b/ui-log.c
index 2d2bb31..f2a7852 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -234,8 +234,11 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
 			strcpy(info->subject + i, wrap_symbol);
 		}
 	}
+	cgit_open_filter(ctx.repo->subject_filter,
+			 oid_to_hex(&commit->object.oid));
 	cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
 			 oid_to_hex(&commit->object.oid), ctx.qry.vpath);
+	cgit_close_filter(ctx.repo->subject_filter);
 	show_commit_decorations(commit);
 	html("</td><td>");
 	cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
-- 8< --

> > What do you think about adding a "filter" which is called before the
> > commit subject is printed and allows outputting additional HTML content?
> > ("filter" is in quotes since there will not be any content copied
> > through the program.)
> 
> Sounds good to me. It covers my use case.
> 
> Perhaps this new "filter" should not have filter in its name.
> Suggestion: commit-prefix=.
> 
> Oh, and this new command must somehow receive the commit id from cgit.
> New environment variable?

We normally use arguments for this, which is all handled via the
cgit_open_filter() function.

I expect Lua filters may be preferred for this use case to avoid forking
50 child processes when displaying the log view.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 12:49     ` john
@ 2017-12-02 14:44       ` 
  2017-12-02 17:10         ` john
  0 siblings, 1 reply; 8+ messages in thread
From:  @ 2017-12-02 14:44 UTC (permalink / raw)


Hi John,

On 2 December 2017 at 13:49, John Keeping <john at keeping.me.uk> wrote:
> After I wrote that I realised that for the existing commit-filter we
> escape the content before writing to the filter.  So it might be
> reasonable to pass the whole <a></a> tag through the filter.
>
> That would make it easy for the filter to put extra content either
> before or after the commit subject.  It also makes the filter
> integration much simpler, for example in ui-log.c:
>
> -- >8 --
> diff --git a/ui-log.c b/ui-log.c
> index 2d2bb31..f2a7852 100644
> --- a/ui-log.c
> +++ b/ui-log.c
> @@ -234,8 +234,11 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
>                         strcpy(info->subject + i, wrap_symbol);
>                 }
>         }
> +       cgit_open_filter(ctx.repo->subject_filter,
> +                        oid_to_hex(&commit->object.oid));
>         cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
>                          oid_to_hex(&commit->object.oid), ctx.qry.vpath);
> +       cgit_close_filter(ctx.repo->subject_filter);
>         show_commit_decorations(commit);
>         html("</td><td>");
>         cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
> -- 8< --

I applied that patch, with s/subject_filter/commit_filter/, and
updated my commit-filter to output a small SVG (borrowed from
https://shields.io/) in front of the commit message.

Here is the result:

https://bforsman.name/cgit/nixos-config/log/

I think it's looking good!

On small annoyance though is that the CI status element, when loaded,
causes the commit messages to move sideways. Ideally, the CI status
(or whatever the user chose to attach to the commit) would not cause
the layout to move (unless it's very big). I guess that's a
conflicting goal between having a "commit prefix" and a dedicated
column for this extra info. Another alternative is _appending_ the
element, which a filter program has the ability to do. Then the layout
would be stable, but the CI status elements would not align nicely in
a column anymore. Hmm... :-)

> We normally use arguments for this, which is all handled via the
> cgit_open_filter() function.
>
> I expect Lua filters may be preferred for this use case to avoid forking
> 50 child processes when displaying the log view.

Good point.

Best regards,
Bj?rn Forsman


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 14:44       ` 
@ 2017-12-02 17:10         ` john
  2017-12-02 17:14           ` 
  0 siblings, 1 reply; 8+ messages in thread
From: john @ 2017-12-02 17:10 UTC (permalink / raw)


On Sat, Dec 02, 2017 at 03:44:15PM +0100, Bjjjrn Forsman wrote:
> On 2 December 2017 at 13:49, John Keeping <john at keeping.me.uk> wrote:
> > After I wrote that I realised that for the existing commit-filter we
> > escape the content before writing to the filter.  So it might be
> > reasonable to pass the whole <a></a> tag through the filter.
> >
> > That would make it easy for the filter to put extra content either
> > before or after the commit subject.  It also makes the filter
> > integration much simpler, for example in ui-log.c:
> >
> > -- >8 --
> > diff --git a/ui-log.c b/ui-log.c
> > index 2d2bb31..f2a7852 100644
> > --- a/ui-log.c
> > +++ b/ui-log.c
> > @@ -234,8 +234,11 @@ static void print_commit(struct commit *commit, struct rev_info *revs)
> >                         strcpy(info->subject + i, wrap_symbol);
> >                 }
> >         }
> > +       cgit_open_filter(ctx.repo->subject_filter,
> > +                        oid_to_hex(&commit->object.oid));
> >         cgit_commit_link(info->subject, NULL, NULL, ctx.qry.head,
> >                          oid_to_hex(&commit->object.oid), ctx.qry.vpath);
> > +       cgit_close_filter(ctx.repo->subject_filter);
> >         show_commit_decorations(commit);
> >         html("</td><td>");
> >         cgit_open_filter(ctx.repo->email_filter, info->author_email, "log");
> > -- 8< --
> 
> I applied that patch, with s/subject_filter/commit_filter/, and
> updated my commit-filter to output a small SVG (borrowed from
> https://shields.io/) in front of the commit message.
> 
> Here is the result:
> 
> https://bforsman.name/cgit/nixos-config/log/
> 
> I think it's looking good!
> 
> On small annoyance though is that the CI status element, when loaded,
> causes the commit messages to move sideways. Ideally, the CI status
> (or whatever the user chose to attach to the commit) would not cause
> the layout to move (unless it's very big). I guess that's a
> conflicting goal between having a "commit prefix" and a dedicated
> column for this extra info. Another alternative is _appending_ the
> element, which a filter program has the ability to do. Then the layout
> would be stable, but the CI status elements would not align nicely in
> a column anymore. Hmm... :-)

I wonder if it would look better with a smaller symbol, like the
Libravatar icons for the author names here:

	https://git.zx2c4.com/cgit/log/

Maybe something like the small tick/cross icons GitHub and GitLab use
for build status, for example:

	https://github.com/rust-lang/rust/branches/all

	https://gitlab.com/gitlab-org/gitlab-ce/commits/master


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 17:10         ` john
@ 2017-12-02 17:14           ` 
  2017-12-02 19:00             ` 
  0 siblings, 1 reply; 8+ messages in thread
From:  @ 2017-12-02 17:14 UTC (permalink / raw)


Hi John,

On 2 December 2017 at 18:10, John Keeping <john at keeping.me.uk> wrote:
> On Sat, Dec 02, 2017 at 03:44:15PM +0100, Bjjjrn Forsman wrote:
>> Here is the result:
>>
>> https://bforsman.name/cgit/nixos-config/log/
>>
>> I think it's looking good!
>
> I wonder if it would look better with a smaller symbol, like the
> Libravatar icons for the author names here:
>
>         https://git.zx2c4.com/cgit/log/
>
> Maybe something like the small tick/cross icons GitHub and GitLab use
> for build status, for example:
>
>         https://github.com/rust-lang/rust/branches/all
>
>         https://gitlab.com/gitlab-org/gitlab-ce/commits/master

Indeed, those look better.

(I didn't plan to use that big/long SVG in the end, it was just the
first I found.)

Best regards,
Bj?rn Forsman


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Commit _subject_ filter, everywhere
  2017-12-02 17:14           ` 
@ 2017-12-02 19:00             ` 
  0 siblings, 0 replies; 8+ messages in thread
From:  @ 2017-12-02 19:00 UTC (permalink / raw)


On 2 December 2017 at 18:14, Bj?rn Forsman <bjorn.forsman at gmail.com> wrote:
> Hi John,
>
> On 2 December 2017 at 18:10, John Keeping <john at keeping.me.uk> wrote:
>> On Sat, Dec 02, 2017 at 03:44:15PM +0100, Bjjjrn Forsman wrote:
>>> Here is the result:
>>>
>>> https://bforsman.name/cgit/nixos-config/log/
>>>
>>> I think it's looking good!
>>
>> I wonder if it would look better with a smaller symbol, like the
>> Libravatar icons for the author names here:
>>
>>         https://git.zx2c4.com/cgit/log/
>>
>> Maybe something like the small tick/cross icons GitHub and GitLab use
>> for build status, for example:
>>
>>         https://github.com/rust-lang/rust/branches/all
>>
>>         https://gitlab.com/gitlab-org/gitlab-ce/commits/master
>
> Indeed, those look better.
>
> (I didn't plan to use that big/long SVG in the end, it was just the
> first I found.)

I did two changes:

1. Borrow SVG from gitlab (thanks for the hint!). Smaller, more
suitable icon. Also, it's a good thing to have a shape that indicates
pass/fail, instead of relying only on color.
2. Specify a fixed size for the SVG in the commit-filter. This
prevents a the page layout from changing when the SVG is finally
loaded.

https://bforsman.name/cgit/nixos-config/log/

The indicator is still listed in the "commit" column, but as it's much
smaller now, I feel that's fine.

Best regards,
Bj?rn Forsman


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-12-02 19:00 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-29 21:10 Commit _subject_ filter, everywhere 
2017-12-02 11:31 ` john
2017-12-02 12:33   ` 
2017-12-02 12:49     ` john
2017-12-02 14:44       ` 
2017-12-02 17:10         ` john
2017-12-02 17:14           ` 
2017-12-02 19:00             ` 

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).