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

[movies] Simplify the driver #71

Merged
merged 5 commits into from
Aug 29, 2021

Conversation

cpitclaudel
Copy link
Contributor

This is a follow-up to #70

With the change that I suggested there, I was able to refactor the movies script into a standard Alectryon pipeline, with just a few custom actions to isolate snippets in the source .v file.

@Casteran I tried to make a minimally invasive patch, so I didn't touch the source files. Instead I just made the code look for existing (*| .. coq:: …flags… |*) annotations and propagate the corresponding flags.

I kept the folder structure, and AFAICT the only differences are spaces; I hope I didn't miss anything (I made it so that spaces before and after snippets get stripped (which is a bit tricky due to (*| .. coq |*) visibility annotations).

This pipeline doesn't go through reST at all. (*| .. coq |*) visibility annotations outside of snippets are ignores. Visibility settings get reset after each snippet, so (*||*) can be useful in the middle of a snippet but is not needed at the end of a snippet.

The syntax (* begin snippet ABC:: …flags… *) is supported but not used (to avoid a larger diff than necessary.

@Zimmi48 I need your help to pull the latest Alectryon.

@Zimmi48
Copy link
Member

Zimmi48 commented Aug 28, 2021

I pushed the same commit as on top of #67 to test the latest master version of Alectryon.

@Casteran
Copy link
Member

Looks nice !
Just a naive question : Do I have to update my (pip installed) version of alectryon on my machine ?

@Zimmi48
Copy link
Member

Zimmi48 commented Aug 28, 2021

Yes, to merge this PR and #67, there are two options:

  1. wait for Clément to make a new release and require it instead of the current 1.3
  2. run pip install from the GitHub repository of Alectryon (and document that this is required).

@Casteran Casteran merged commit acb4519 into coq-community:master Aug 29, 2021
@Zimmi48 Zimmi48 mentioned this pull request Aug 31, 2021
@Zimmi48
Copy link
Member

Zimmi48 commented Sep 8, 2021

Something I've just noticed is that ever since this PR, the "Build Alectryon snippets" step in the build-doc job does nothing and all the snippet building is actually done in the "Compile LaTeX document" step. I don't really understand why that would be the case given that the Makefile in doc/movies is still there.

@cpitclaudel
Copy link
Contributor Author

I don't really understand why that would be the case given that the Makefile in doc/movies is still there.

Thanks for catching that, fixed in #84

@cpitclaudel cpitclaudel deleted the simple_driver branch September 24, 2021 05:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants