Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split out lexicon, plugins and config page content #3195

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mauriceyap
Copy link
Member

In the documentation site, extract out the pre-templater content of the pages, lexicon.html, plugins.html and config.html into separate build targets, so that they can be used to build out a search index. In the search index, we do not want the content which the templater adds from template.html.

I have tested this by ensuring that these three generated templated pages before, and after these changes are the same:

$ cp $(plz build //docs:lexicon_html) tmp/lexicon_old.html
$ cp $(plz build //docs:config_html) tmp/config_old.html
$ cp $(plz build //docs:plugins_html) tmp/plugins_old.html

# Make the changes

$ cp $(plz build //docs:lexicon_html) tmp/lexicon_new.html
$ cp $(plz build //docs:config_html) tmp/config_new.html
$ cp $(plz build //docs:plugins_html) tmp/plugins_new.html

$ diff tmp/lexicon_old.html tmp/lexicon_new.html  # no output
$ diff tmp/config_old.html tmp/config_new.html # no output
$ diff tmp/plugins_old.html tmp/plugins_new.html # no output

docs/BUILD Outdated
outs = ["lexicon.html"],
cmd = [
'"$TOOLS_LEX" -i docs/lexicon.html -i docs/lexicon_entry.html docs/rules.json > "$OUT"',
'"$TOOLS_TMPL" --template docs/template.html --in lexicon.html > tmp.html',
'mv docs/lexicon_content.html "$OUT"',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

And more accurately, why not:

genrule(
    name = "lexicon_html",
    srcs = deps + [
        ":lexicon_content",
        "template.html",
    ],
    outs = ["lexicon.html"],
    cmd = [
        '"$TOOL" --template docs/template.html --in docs/lexicon_content.html > "$OUT"',
    ],
    tools = ["//docs/tools/templater"],
    visibility = ["//docs/test/..."],
)

similarly below.

I'm not a fan of how the command relies on filenames rather than using $SRCS, but I'm guessing the template implicitly depends on the files in deps, which makes it hard to use the dict-style srcs

docs/BUILD Outdated
@@ -80,43 +89,62 @@ filegroup(
)

genrule(
name = "plugins_html",
name = "plugins_content",
srcs = deps + [
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You removed deps in the other two _content targets?

Copy link
Member

@peterebden peterebden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I've mentioned on #3192 , I'm unclear that building a search index ourselves is the right approach to take. We should agree on the approach to take before making more code changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants