X-Git-Url: http://git.vanrenterghem.biz/git.ikiwiki.info.git/blobdiff_plain/fb4a4e86335f21e88fe5eea763fb9dcdbe58492f..6975f9e3f4e06fb55cc8c9795c503019d2e8d08b:/doc/todo/structured_page_data.mdwn?ds=sidebyside
diff --git a/doc/todo/structured_page_data.mdwn b/doc/todo/structured_page_data.mdwn
index 2b8309a67..2a196ed23 100644
--- a/doc/todo/structured_page_data.mdwn
+++ b/doc/todo/structured_page_data.mdwn
@@ -45,10 +45,10 @@ This seems conceptually straightforward, if possibly quite internally
complex to handle the more complicated types and validation.
One implementation wrinkle is how to build the html form. The editpage.tmpl
-currently overrides the standard [[cpan CGI::FormBuilder]] generated form,
+currently overrides the standard [[!cpan CGI::FormBuilder]] generated form,
which was done to make the edit page be laid out in a nice way. This,
however, means that new fields cannot be easily added to it using
-[[cpan CGI::FormBuilder]]. The attachment plugin uses the hack of bouilding
+[[!cpan CGI::FormBuilder]]. The attachment plugin uses the hack of bouilding
up html by hand and dumping it into the form via a template variable.
It would be nice if the type implementation code could just use
@@ -103,3 +103,521 @@ See also:
>
>Anyway, I just wanted to list the thoughts. In none of these use cases is straight yaml or json the
>obvious answer. -- [[Will]]
+
+>> Okie. I've had a play with this. A 'form' plugin is included inline below, but it is only a rough first pass to
+>> get a feel for the design space.
+>>
+>> The current design defines a new type of page - a 'form'. The type of page holds YAML data
+>> defining a FormBuilder form. For example, if we add a file to the wiki source `test.form`:
+
+ ---
+ fields:
+ age:
+ comment: This is a test
+ validate: INT
+ value: 15
+
+>> The YAML content is a series of nested hashes. The outer hash is currently checked for two keys:
+>> 'template', which specifies a parameter to pass to the FromBuilder as the template for the
+>> form, and 'fields', which specifies the data for the fields on the form.
+>> each 'field' is itself a hash. The keys and values are arguments to the formbuilder form method.
+>> The most important one is 'value', which specifies the value of that field.
+>>
+>> Using this, the plugin below can output a form when asked to generate HTML. The Formbuilder
+>> arguments are sanitized (need a thorough security audit here - I'm sure I've missed a bunch of
+>> holes). The form is generated with default values as supplied in the YAML data. It also has an
+>> 'Update Form' button at the bottom.
+>>
+>> The 'Update Form' button in the generated HTML submits changed values back to IkiWiki. The
+>> plugin captures these new values, updates the YAML and writes it out again. The form is
+>> validated when edited using this method. This method can only edit the values in the form.
+>> You cannot add new fields this way.
+>>
+>> It is still possible to edit the YAML directly using the 'edit' button. This allows adding new fields
+>> to the form, or adding other formbuilder data to change how the form is displayed.
+>>
+>> One final part of the plugin is a new pagespec function. `form_eq()` is a pagespec function that
+>> takes two arguments (separated by a ','). The first argument is a field name, the second argument
+>> a value for that field. The function matches forms (and not other page types) where the named
+>> field exists and holds the value given in the second argument. For example:
+
+ \[[!inline pages="form_eq(age,15)" archive="yes"]]
+
+>> will include a link to the page generated above.
+
+>>> Okie, I've just made another plugin to try and do things in a different way.
+>>> This approach adds a 'data' directive. There are two arguments, `key` and `value`.
+>>> The directive is replaced by the value. There is also a match function, which is similar
+>>> to the one above. It also takes two arguments, a key and a value. It returns true if the
+>>> page has that key/value pair in a data directive. e.g.:
+
+ \[[!data key="age" value="15"]]
+
+>>> then, in another page:
+
+ \[[!inline pages="data_eq(age,15)" archive="yes"]]
+
+>>> I expect that we could have more match functions for each type of structured data,
+>>> I just wanted to implement a rough prototype to get a feel for how it behaves. -- [[Will]]
+
+>> Anyway, here are the plugins. As noted above these are only preliminary, exploratory, attempts. -- [[Will]]
+
+>>>> I've just updated the second of the two patches below. The two patches are not mutually
+>>>> exclusive, but I'm leaning towards the second as more useful (for the things I'm doing). -- [[Will]]
+
+I think it's awesome that you're writing this code to explore the problem
+space, [[Will]] -- and these plugins are good stabs at at least part of it.
+Let me respond to a few of your comments.. --[[Joey]]
+
+On use cases, one use case is a user posting a bug report with structured
+data in it. A template is one way, but then the user has to deal with the
+format used to store the structured data. This is where a edit-time form
+becomes essential.
+
+> This was the idea with the 'form' plugin. With the 'data' plugin I was exploring
+> a different approach: try to keep the markup simple enough that the user can edit
+> the markup directly, and still have that be ok. I admit it is a stretch, but I thought
+> it worth exploring.
+
+Another use case is, after many such bugs have been filed,
+wanting to add a new field to each bug report. To avoid needing to edit
+every bug report it would be good if the fields in a bug report were
+defined somewhere else, so that just that one place can be edited to add
+the new field, and it will show up in each bug report (and in each bug
+report's edit page, as a new form field).
+
+> If I was going to do that, I'd use a perl script on a checked out
+> workspace. I think you're describing a rare operation and
+> so I'd be happy not having a web interface for it. Having said that,
+> if you just wanted to change the form for *new* pages, then you
+> can just edit the template used to create new pages.
+
+Re the form plugin, I'm uncomfortable with tying things into
+[[!cpan CGI::FormBuilder]] quite so tightly as you have.
+
+> Yeah :). But I wanted to explore the space and that was the
+> easiest way to start.
+
+CGI::FormBuilder
+could easily change in a way that broke whole wikis full of pages. Also,
+needing to sanitize FormBuilder fields with security implications is asking
+for trouble, since new FormBuilder features could add new fields, or
+add new features to existing fields (FormBuilder is very DWIM) that open
+new security holes.
+
+> There is a list of allowed fields. I only interpret those.
+
+I think that having a type system, that allows defining specific types,
+like "email address", by writing code (that in turn can use FormBuilder),
+is a better approach, since it should avoid becoming a security problem.
+
+> That would be possible. I think an extension to the 'data' plugin might
+> work here.
+
+One specific security hole, BTW, is that if you allow the `validate` field,
+FormBuilder will happily treat it as a regexp, and we don't want to expose
+arbitrary perl regexps, since they can at least DOS a system, and can
+probably be used to run arbitrary perl code.
+
+> I validate the validate field :). It only allows validate fields that match
+> `/^[\w\s]+$/`. This means you can really only use the pre-defined
+> validation types in FormBuilder.
+
+The data plugin only deals with a fairly small corner of the problem space,
+but I think does a nice job at what it does. And could probably be useful
+in a large number of other cases.
+
+> I think the data plugin is more likely to be useful than the form plugin.
+> I was thinking of extending the data directive by allowing an 'id' parameter.
+> When you have an id parameter, then you can display a small form for that
+> data element. The submission handler would look through the page source
+> for the data directive with the right id parameter and edit it. This would
+> make the data directive more like the current 'form' plugin.
+
+> That is making things significantly more complex for less significant gain though. --[[Will]]
+
+> Oh, one quick other note. The data plugin below was designed to handle multiple
+> data elements in a single directive. e.g.
+
+ \[[!data key="Depends on" link="bugs/bugA" link="bugs/bugB" value=6]]
+
+> would match `data_eq(Depends on,6)`, `data_link(Depends on,bugs/bugA)`, `data_link(Depends on,bugs/bugB)`
+> or, if you applied the patch in [[todo/tracking_bugs_with_dependencies]] then you can use 'defined pagespecs'
+> such as `data_link(Depends on,~openBugs)`. The ability to label links like this allows separation of
+> dependencies between bugs from arbitrary links.
+
+----
+
+ #!/usr/bin/perl
+ # Interpret YAML data to make a web form
+ package IkiWiki::Plugin::form;
+
+ use warnings;
+ use strict;
+ use CGI::FormBuilder;
+ use IkiWiki 2.00;
+
+ sub import { #{{{
+ hook(type => "getsetup", id => "form", call => \&getsetup);
+ hook(type => "htmlize", id => "form", call => \&htmlize);
+ hook(type => "sessioncgi", id => "form", call => \&cgi_submit);
+ } # }}}
+
+ sub getsetup () { #{{{
+ return
+ plugin => {
+ safe => 1,
+ rebuild => 1, # format plugin
+ },
+ } #}}}
+
+ sub makeFormFromYAML ($$$) { #{{{
+ my $page = shift;
+ my $YAMLString = shift;
+ my $q = shift;
+
+ eval q{use YAML};
+ error($@) if $@;
+ eval q{use CGI::FormBuilder};
+ error($@) if $@;
+
+ my ($dataHashRef) = YAML::Load($YAMLString);
+
+ my @fields = keys %{ $dataHashRef->{fields} };
+
+ unshift(@fields, 'do');
+ unshift(@fields, 'page');
+ unshift(@fields, 'rcsinfo');
+
+ # print STDERR "Fields: @fields\n";
+
+ my $submittedPage;
+
+ $submittedPage = $q->param('page') if defined $q;
+
+ if (defined $q && defined $submittedPage && ! ($submittedPage eq $page)) {
+ error("Submitted page doensn't match current page: $page, $submittedPage");
+ }
+
+ error("Page not backed by file") unless defined $pagesources{$page};
+ my $file = $pagesources{$page};
+
+ my $template;
+
+ if (defined $dataHashRef->{template}) {
+ $template = $dataHashRef->{template};
+ } else {
+ $template = "form.tmpl";
+ }
+
+ my $form = CGI::FormBuilder->new(
+ fields => \@fields,
+ charset => "utf-8",
+ method => 'POST',
+ required => [qw{page}],
+ params => $q,
+ action => $config{cgiurl},
+ template => scalar IkiWiki::template_params($template),
+ wikiname => $config{wikiname},
+ header => 0,
+ javascript => 0,
+ keepextras => 0,
+ title => $page,
+ );
+
+ $form->field(name => 'do', value => 'Update Form', required => 1, force => 1, type => 'hidden');
+ $form->field(name => 'page', value => $page, required => 1, force => 1, type => 'hidden');
+ $form->field(name => 'rcsinfo', value => IkiWiki::rcs_prepedit($file), required => 1, force => 0, type => 'hidden');
+
+ my %validkey;
+ foreach my $x (qw{label type multiple value fieldset growable message other required validate cleanopts columns comment disabled linebreaks class}) {
+ $validkey{$x} = 1;
+ }
+
+ while ( my ($name, $data) = each(%{ $dataHashRef->{fields} }) ) {
+ next if $name eq 'page';
+ next if $name eq 'rcsinfo';
+
+ while ( my ($key, $value) = each(%{ $data }) ) {
+ next unless $validkey{$key};
+ next if $key eq 'validate' && !($value =~ /^[\w\s]+$/);
+
+ # print STDERR "Adding to field $name: $key => $value\n";
+ $form->field(name => $name, $key => $value);
+ }
+ }
+
+ # IkiWiki::decode_form_utf8($form);
+
+ return $form;
+ } #}}}
+
+ sub htmlize (@) { #{{{
+ my %params=@_;
+ my $content = $params{content};
+ my $page = $params{page};
+
+ my $form = makeFormFromYAML($page, $content, undef);
+
+ return $form->render(submit => 'Update Form');
+ } # }}}
+
+ sub cgi_submit ($$) { #{{{
+ my $q=shift;
+ my $session=shift;
+
+ my $do=$q->param('do');
+ return unless $do eq 'Update Form';
+ IkiWiki::decode_cgi_utf8($q);
+
+ eval q{use YAML};
+ error($@) if $@;
+ eval q{use CGI::FormBuilder};
+ error($@) if $@;
+
+ my $page = $q->param('page');
+
+ return unless exists $pagesources{$page};
+
+ return unless $pagesources{$page} =~ m/\.form$/ ;
+
+ return unless IkiWiki::check_canedit($page, $q, $session);
+
+ my $file = $pagesources{$page};
+ my $YAMLString = readfile(IkiWiki::srcfile($file));
+ my $form = makeFormFromYAML($page, $YAMLString, $q);
+
+ my ($dataHashRef) = YAML::Load($YAMLString);
+
+ if ($form->submitted eq 'Update Form' && $form->validate) {
+
+ #first update our data structure
+
+ while ( my ($name, $data) = each(%{ $dataHashRef->{fields} }) ) {
+ next if $name eq 'page';
+ next if $name eq 'rcs-data';
+
+ if (defined $q->param($name)) {
+ $data->{value} = $q->param($name);
+ }
+ }
+
+ # now write / commit the data
+
+ writefile($file, $config{srcdir}, YAML::Dump($dataHashRef));
+
+ my $message = "Web form submission";
+
+ IkiWiki::disable_commit_hook();
+ my $conflict=IkiWiki::rcs_commit($file, $message,
+ $form->field("rcsinfo"),
+ $session->param("name"), $ENV{REMOTE_ADDR});
+ IkiWiki::enable_commit_hook();
+ IkiWiki::rcs_update();
+
+ require IkiWiki::Render;
+ IkiWiki::refresh();
+
+ IkiWiki::redirect($q, "$config{url}/".htmlpage($page)."?updated");
+
+ } else {
+ error("Invalid data!");
+ }
+
+ exit;
+ } #}}}
+
+ package IkiWiki::PageSpec;
+
+ sub match_form_eq ($$;@) { #{{{
+ my $page=shift;
+ my $argSet=shift;
+ my @args=split(/,/, $argSet);
+ my $field=shift @args;
+ my $value=shift @args;
+
+ my $file = $IkiWiki::pagesources{$page};
+
+ if ($file !~ m/\.form$/) {
+ return IkiWiki::FailReason->new("page is not a form");
+ }
+
+ my $YAMLString = IkiWiki::readfile(IkiWiki::srcfile($file));
+
+ eval q{use YAML};
+ error($@) if $@;
+
+ my ($dataHashRef) = YAML::Load($YAMLString);
+
+ if (! defined $dataHashRef->{fields}->{$field}) {
+ return IkiWiki::FailReason->new("field '$field' not defined in page");
+ }
+
+ my $formVal = $dataHashRef->{fields}->{$field}->{value};
+
+ if ($formVal eq $value) {
+ return IkiWiki::SuccessReason->new("field value matches");
+ } else {
+ return IkiWiki::FailReason->new("field value does not match");
+ }
+ } #}}}
+
+ 1
+
+----
+
+ #!/usr/bin/perl
+ # Allow data embedded in a page to be checked for
+ package IkiWiki::Plugin::data;
+
+ use warnings;
+ use strict;
+ use IkiWiki 2.00;
+
+ my $inTable = 0;
+
+ sub import { #{{{
+ hook(type => "getsetup", id => "data", call => \&getsetup);
+ hook(type => "needsbuild", id => "data", call => \&needsbuild);
+ hook(type => "preprocess", id => "data", call => \&preprocess, scan => 1);
+ hook(type => "preprocess", id => "datatable", call => \&preprocess_table, scan => 1); # does this need scan?
+ } # }}}
+
+ sub getsetup () { #{{{
+ return
+ plugin => {
+ safe => 1,
+ rebuild => 1, # format plugin
+ },
+ } #}}}
+
+ sub needsbuild (@) { #{{{
+ my $needsbuild=shift;
+ foreach my $page (keys %pagestate) {
+ if (exists $pagestate{$page}{data}) {
+ if (exists $pagesources{$page} &&
+ grep { $_ eq $pagesources{$page} } @$needsbuild) {
+ # remove state, it will be re-added
+ # if the preprocessor directive is still
+ # there during the rebuild
+ delete $pagestate{$page}{data};
+ }
+ }
+ }
+ }
+
+ sub preprocess (@) { #{{{
+ my @argslist = @_;
+ my %params=@argslist;
+
+ my $html = '';
+ my $class = defined $params{class}
+ ? 'class="'.$params{class}.'"'
+ : '';
+
+ if ($inTable) {
+ $html = "| $params{key}: | ";
+ } else {
+ $html = "$params{key}:";
+ }
+
+ while (scalar(@argslist) > 1) {
+ my $type = shift @argslist;
+ my $data = shift @argslist;
+ if ($type eq 'link') {
+ # store links raw
+ $pagestate{$params{page}}{data}{$params{key}}{link}{$data} = 1;
+ my $link=IkiWiki::linkpage($data);
+ add_depends($params{page}, $link);
+ $html .= ' ' . htmllink($params{page}, $params{destpage}, $link);
+ } elsif ($type eq 'data') {
+ $data = IkiWiki::preprocess($params{page}, $params{destpage},
+ IkiWiki::filter($params{page}, $params{destpage}, $data));
+ $html .= ' ' . $data;
+ # store data after processing - allows pagecounts to be stored, etc.
+ $pagestate{$params{page}}{data}{$params{key}}{data}{$data} = 1;
+ }
+ }
+
+ if ($inTable) {
+ $html .= " | ";
+ } else {
+ $html .= "";
+ }
+
+ return $html;
+ } # }}}
+
+ sub preprocess_table (@) { #{{{
+ my %params=@_;
+
+ my @lines;
+ push @lines, defined $params{class}
+ ? "'
+ : '';
+
+ $inTable = 1;
+
+ foreach my $line (split(/\n/, $params{datalist})) {
+ push @lines, "" . IkiWiki::preprocess($params{page}, $params{destpage},
+ IkiWiki::filter($params{page}, $params{destpage}, $line)) . "
";
+ }
+
+ $inTable = 0;
+
+ push @lines, '
';
+
+ return join("\n", @lines);
+ } #}}}
+
+ package IkiWiki::PageSpec;
+
+ sub match_data_eq ($$;@) { #{{{
+ my $page=shift;
+ my $argSet=shift;
+ my @args=split(/,/, $argSet);
+ my $key=shift @args;
+ my $value=shift @args;
+
+ if (! exists $IkiWiki::pagestate{$page}{data}) {
+ return IkiWiki::FailReason->new("page does not contain any data directives");
+ }
+
+ if (! exists $IkiWiki::pagestate{$page}{data}{$key}) {
+ return IkiWiki::FailReason->new("page does not contain data key '$key'");
+ }
+
+ if ($IkiWiki::pagestate{$page}{data}{$key}{data}{$value}) {
+ return IkiWiki::SuccessReason->new("value matches");
+ } else {
+ return IkiWiki::FailReason->new("value does not match");
+ }
+ } #}}}
+
+ sub match_data_link ($$;@) { #{{{
+ my $page=shift;
+ my $argSet=shift;
+ my @params=@_;
+ my @args=split(/,/, $argSet);
+ my $key=shift @args;
+ my $value=shift @args;
+
+ if (! exists $IkiWiki::pagestate{$page}{data}) {
+ return IkiWiki::FailReason->new("page $page does not contain any data directives and so cannot match a link");
+ }
+
+ if (! exists $IkiWiki::pagestate{$page}{data}{$key}) {
+ return IkiWiki::FailReason->new("page $page does not contain data key '$key'");
+ }
+
+ foreach my $link (keys %{ $IkiWiki::pagestate{$page}{data}{$key}{link} }) {
+ # print STDERR "Checking if $link matches glob $value\n";
+ if (match_glob($link, $value, @params)) {
+ return IkiWiki::SuccessReason->new("Data link on page $page with key $key matches glob $value: $link");
+ }
+ }
+
+ return IkiWiki::FailReason->new("No data link on page $page with key $key matches glob $value");
+ } #}}}
+
+ 1