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

[ close #3373 ] Implement the proposed feature #3384

Merged
merged 3 commits into from
Nov 13, 2018

Conversation

ice1000
Copy link
Member

@ice1000 ice1000 commented Nov 13, 2018

Signed-off-by: ice1000 ice1000kotlin@foxmail.com

Everything has already been covered in #3373

@ice1000 ice1000 added the backend: html HTML generation backend label Nov 13, 2018
@ice1000 ice1000 requested a review from vlopezj November 13, 2018 02:23
@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

We can try render the user manual with our new HTML backend with --html-highlight=auto!

@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

screenshot_2018-11-13-02-49-59-78

Makes my day

@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

@vlopezj say something?

Just curious, what's your timezone? It's 4:48 a.m. here.

@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

CI passed! Yay!

@vlopezj
Copy link
Contributor

vlopezj commented Nov 13, 2018

@ice1000 It looks good; but I'll look at it in more detail this afternoon (I'm in Europe/Stockholm).

We can try render the user manual with our new HTML backend with --html-highlight=auto!

Yes, this is the way to go. There are however two issues that make it non-straightforward:

  • We would need to process the .rst files for the documentation for each commit, using that commit's version of Agda. I don't know this would fit with the current Readthedocs building methodology.

    https://docs.readthedocs.io/en/latest/builds.html#how-we-build-documentation

  • Read the docs also generates PDF files, which I assume do not support the HTML blocks.
    (But this is fine, we can keep generating the PDF from the original .lagda.rst files).

@gallais
Copy link
Member

gallais commented Nov 13, 2018

I don't know this would fit with the current Readthedocs building methodology.

Note that we could push the generated html to github and have it sit at agda.github.io.
We already do something similar for the standard library

@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

We would need to process the .rst files for the documentation for each commit, using that commit's version of Agda. I don't know this would fit with the current Readthedocs building methodology.

It shouldn't be a big problem to put the code-rendered rst files into the repo. It's possible to let Travis CI do this (although I don't know how to let Travis CI push commits into a repo without using a GitHub password). Why do we need to do it for each commit? Do you mean each commit since the very beginning or from now on?

@@ -20,8 +20,7 @@ highlightOnlyCode HighlightAll _ = False
highlightOnlyCode HighlightCode _ = True
highlightOnlyCode HighlightAuto AgdaFileType = False
highlightOnlyCode HighlightAuto MdFileType = True
-- TODO: see #3373
highlightOnlyCode HighlightAuto RstFileType = False
highlightOnlyCode HighlightAuto RstFileType = True
Copy link
Contributor

Choose a reason for hiding this comment

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

The functions highlightOnlyCode and highlightedFileExt are specific to the HTML backend. They should go in Agda.Interaction.Highlighting.HTML.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure! I thought it'll be used somewhere else when I first impl these but seems not.

@vlopezj
Copy link
Contributor

vlopezj commented Nov 13, 2018

As @gallais pointed out, it seems we are already doing essentially the same thing for the Agda standard library (generating HTML files from Agda and pushing them to github pages). For the user manual, we have two steps: generating .rst files, and then generating .html files from the .rst files.

It seems that this can be done with Travis. To avoid the need for a Github password, I think you can generate some form of token, give it to Travis, and then reference it from the .travis.yml file:

https://github.com/agda/agda-stdlib/blob/master/.travis.yml

If we have this, I think it will be easier to bypass read the docs completely and just publish HTML files directly under https://agda.github.io/docs .

@ice1000 In any case, the first step would be to have a Makefile rule (such as make user-manual-html-highlighted) that can actually generate the .rst files with the embedded HTML, and then build the HTML manual from them. Putting the result online will then be easy.

@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

@ice1000 In any case, the first step would be to have a Makefile rule (such as make user-manual-html-highlighted) that can actually generate the .rst files with the embedded HTML, and then build the HTML manual from them. Putting the result online will then be easy.

IMO It's a separated task. I like this idea but I'm not gonma do that in this PR.

@vlopezj
Copy link
Contributor

vlopezj commented Nov 13, 2018

IMO It's a separated task. I like this idea but I'm not gonma do that in this PR.

Makes sense.

Then it looks ready for merging (apart from the comment about highlightOnlyCode and highlightedFileExt). I assume the end result will be a single commit. You can rebase it on master instead of merging if you want to avoid creating an extra merge commit.

Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
…ight=auto`

Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
Signed-off-by: ice1000 <ice1000kotlin@foxmail.com>
@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

@vlopezj Rebased on master, Added changelog, documentation and addressed your comment. Please take a look.

@ice1000 ice1000 merged commit 2d98089 into agda:master Nov 13, 2018
@ice1000 ice1000 deleted the rst-raw-derictive branch November 13, 2018 17:11
@ice1000
Copy link
Member Author

ice1000 commented Nov 13, 2018

CI failed because I deleted the branch too fast and they failed to pull them and build 😂
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: html HTML generation backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants