[PATCH] filter: add support for owner-filter
John Keeping
john at keeping.me.uk
Fri Aug 1 20:18:50 CEST 2014
On Fri, Aug 01, 2014 at 01:54:46PM -0400, Chris Burroughs wrote:
> Followup and attempt at implementation of an owner filter as described
> in http://lists.zx2c4.com/pipermail/cgit/2014-July/002163.html. There
> are two parts I am unsure of:
> * Is there an advantage (performance?) to passing the owner as an
> argument instead of using CGIT_REPO_OWNER?
I suspect argument parsing is quicker than a lookup in the environment,
particularly with Lua filters, but without benchmarking I wouldn't like
to say.
> * I passed along the cgit_rooturl so that a filter could re-implement
> what the C code is doing. This seemed a good test of if the filter
> was flexible enough, but passing it along as an argument feels a
> bit wonky. Maybe this would be better as an environmental variable
> to all filters?
Possibly, but see below...
> ---
> cgit.c | 6 ++++++
> cgit.h | 4 +++-
> cgitrc.5.txt | 18 ++++++++++++++++++
> filter.c | 6 ++++++
> filters/owner-example.lua | 24 ++++++++++++++++++++++++
> shared.c | 1 +
> ui-repolist.c | 2 ++
> 7 files changed, 60 insertions(+), 1 deletion(-)
> create mode 100644 filters/owner-example.lua
>
> diff --git a/cgit.c b/cgit.c
> index 20f6e27..d29e7f5 100644
> --- a/cgit.c
> +++ b/cgit.c
> @@ -91,6 +91,8 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
> repo->source_filter = cgit_new_filter(value, SOURCE);
> else if (!strcmp(name, "email-filter"))
> repo->email_filter = cgit_new_filter(value, EMAIL);
> + else if (!strcmp(name, "owner-filter"))
> + repo->owner_filter = cgit_new_filter(value, OWNER);
> }
> }
>
> @@ -194,6 +196,8 @@ static void config_cb(const char *name, const char *value)
> ctx.cfg.commit_filter = cgit_new_filter(value, COMMIT);
> else if (!strcmp(name, "email-filter"))
> ctx.cfg.email_filter = cgit_new_filter(value, EMAIL);
> + else if (!strcmp(name, "owner-filter"))
> + ctx.cfg.owner_filter = cgit_new_filter(value, OWNER);
> else if (!strcmp(name, "auth-filter"))
> ctx.cfg.auth_filter = cgit_new_filter(value, AUTH);
> else if (!strcmp(name, "embedded"))
> @@ -802,6 +806,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
> cgit_fprintf_filter(repo->source_filter, f, "repo.source-filter=");
> if (repo->email_filter && repo->email_filter != ctx.cfg.email_filter)
> cgit_fprintf_filter(repo->email_filter, f, "repo.email-filter=");
> + if (repo->owner_filter && repo->owner_filter != ctx.cfg.owner_filter)
> + cgit_fprintf_filter(repo->owner_filter, f, "repo.owner-filter=");
> if (repo->snapshots != ctx.cfg.snapshots) {
> char *tmp = build_snapshot_setting(repo->snapshots);
> fprintf(f, "repo.snapshots=%s\n", tmp ? tmp : "");
> diff --git a/cgit.h b/cgit.h
> index 0badc64..0078ac1 100644
> --- a/cgit.h
> +++ b/cgit.h
> @@ -53,7 +53,7 @@ typedef void (*filepair_fn)(struct diff_filepair *pair);
> typedef void (*linediff_fn)(char *line, int len);
>
> typedef enum {
> - ABOUT, COMMIT, SOURCE, EMAIL, AUTH
> + ABOUT, COMMIT, SOURCE, EMAIL, AUTH, OWNER
> } filter_type;
>
> struct cgit_filter {
> @@ -100,6 +100,7 @@ struct cgit_repo {
> struct cgit_filter *commit_filter;
> struct cgit_filter *source_filter;
> struct cgit_filter *email_filter;
> + struct cgit_filter *owner_filter;
> struct string_list submodules;
> };
>
> @@ -253,6 +254,7 @@ struct cgit_config {
> struct cgit_filter *commit_filter;
> struct cgit_filter *source_filter;
> struct cgit_filter *email_filter;
> + struct cgit_filter *owner_filter;
> struct cgit_filter *auth_filter;
> };
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index b7570db..7ed2c80 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -247,6 +247,14 @@ logo-link::
> calculated url of the repository index page will be used. Default
> value: none.
>
> +owner-filter::
> + Specifies a command which will be invoked to format the Owner column
> + of the main page. The command will get the default html on its STDIN,
> + and the STDOUT from the command will be included verbatim in the
> + table. This can be used to link to additional context such as an
> + owners home page. Default value: none.
> + See also: "FILTER API".
> +
> max-atom-items::
> Specifies the number of items to display in atom feeds view. Default
> value: "10".
> @@ -509,6 +517,10 @@ repo.logo-link::
> calculated url of the repository index page will be used. Default
> value: global logo-link.
>
> +repo.owner-filter::
> + Override the default owner-filter. Default value: none. See also:
> + "enable-filter-overrides". See also: "FILTER API".
> +
> repo.module-link::
> Text which will be used as the formatstring for a hyperlink when a
> submodule is printed in a directory listing. The arguments for the
> @@ -641,6 +653,12 @@ email filter::
> expected to write to standard output the formatted text to be included
> in the page.
>
> +owner filter::
> + This filter is given two parameters: the repository owner and the cgit
> + root url. The filter will then receive the default html to argument
> + on standard input and is expected to write to standard output the
> + formatted text to be included in the column.
> +
> source filter::
> This filter is given a single parameter: the filename of the source
> file to filter. The filter can use the filename to determine (for
> diff --git a/filter.c b/filter.c
> index 270f009..35502b5 100644
> --- a/filter.c
> +++ b/filter.c
> @@ -38,12 +38,14 @@ void cgit_cleanup_filters(void)
> reap_filter(ctx.cfg.commit_filter);
> reap_filter(ctx.cfg.source_filter);
> reap_filter(ctx.cfg.email_filter);
> + reap_filter(ctx.cfg.owner_filter);
> reap_filter(ctx.cfg.auth_filter);
> for (i = 0; i < cgit_repolist.count; ++i) {
> reap_filter(cgit_repolist.repos[i].about_filter);
> reap_filter(cgit_repolist.repos[i].commit_filter);
> reap_filter(cgit_repolist.repos[i].source_filter);
> reap_filter(cgit_repolist.repos[i].email_filter);
> + reap_filter(cgit_repolist.repos[i].owner_filter);
> }
> }
>
> @@ -425,6 +427,10 @@ struct cgit_filter *cgit_new_filter(const char *cmd, filter_type filtertype)
> argument_count = 2;
> break;
>
> + case OWNER:
> + argument_count = 2;
> + break;
> +
> case SOURCE:
> case ABOUT:
> argument_count = 1;
> diff --git a/filters/owner-example.lua b/filters/owner-example.lua
> new file mode 100644
> index 0000000..79b1d49
> --- /dev/null
> +++ b/filters/owner-example.lua
> @@ -0,0 +1,24 @@
> +-- This script is an example of an owner-filter. It adds an
> +-- additional link to a fictional home page. This script may be used
> +-- with the owner-filter or repo.owner-filter settings in cgitrc with
> +-- the `lua:` prefix. `cgit_root_url` is provided so that it's
> +-- possible to replicate the default behavior with a filter instead of
> +-- echoing the buffer.
> +--
> +
> +function filter_open(cgit_repo_owner, cgit_root_url)
> + buffer = ""
> + owner = cgit_repo_owner
> + root_url = cgit_root_url
> +end
> +
> +function filter_close()
> + html(buffer)
> + html(string.format(" <a href=\"%s\">%s</a>", "http://wiki.example.com/about/" .. owner, "Home"))
> + return 0
> +end
> +
> +function filter_write(str)
> + buffer = buffer .. str
> +end
This isn't really acting as a filter, you're taking the argument from
the function call and then just appending something based on that from
the default generated by the C code. I think the email filter has to
take the email address as an argument because it may not be present in
the input to the filter (depending on the value of "noplainemail")...
> diff --git a/shared.c b/shared.c
> index 8ed14c0..6e91857 100644
> --- a/shared.c
> +++ b/shared.c
> @@ -72,6 +72,7 @@ struct cgit_repo *cgit_add_repo(const char *url)
> ret->commit_filter = ctx.cfg.commit_filter;
> ret->source_filter = ctx.cfg.source_filter;
> ret->email_filter = ctx.cfg.email_filter;
> + ret->owner_filter = ctx.cfg.owner_filter;
> ret->clone_url = ctx.cfg.clone_url;
> ret->submodules.strdup_strings = 1;
> return ret;
> diff --git a/ui-repolist.c b/ui-repolist.c
> index c2bcce1..8997d08 100644
> --- a/ui-repolist.c
> +++ b/ui-repolist.c
> @@ -306,6 +306,7 @@ void cgit_print_repolist()
> html_link_close();
> html("</td><td>");
> if (ctx.cfg.enable_index_owner) {
> + cgit_open_filter(ctx.repo->owner_filter, ctx.repo->owner, cgit_rooturl());
> html("<a href='");
> html_attr(cgit_rooturl());
> html("?q=");
> @@ -313,6 +314,7 @@ void cgit_print_repolist()
> html("'>");
> html_txt(ctx.repo->owner);
> html("</a>");
> + cgit_close_filter(ctx.repo->owner_filter);
Perhaps this would be better as:
if (ctx.cfg.enable_index_owner) {
if (ctx.repo->owner_filter) {
cgit_open_filter(ctx.repo->owner_filter);
html_txt(ctx.repo->owner);
cgit_close_filter(ctx.repo->owner_filter);
} else {
html("<a href='");
html_attr(cgit_rooturl());
html("?=");
html_url_arg(ctx.repo->owner);
html("'>");
html_txt(ctx.repo->owner);
html("</a>");
}
html("</td><td>");
}
so that the filter really is a filter acting on the owner. Then the Lua
implementation becomes:
-- >8 --
function filter_open()
buffer = ""
end
function filter_close()
html(string.format("<a href=\"%s\">%s</a>",
"http://wiki.example.com/about/" .. buffer, buffer))
return 0
end
function filter_write(str)
buffer = buffer .. str
end
-- 8< --
What do you think?
> html("</td><td>");
> }
> print_modtime(ctx.repo);
More information about the CGit
mailing list