Skip to content

Inlined documentation #783

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

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open

Inlined documentation #783

wants to merge 23 commits into from

Conversation

strub
Copy link
Member

@strub strub commented May 26, 2025

No description provided.

@strub strub requested a review from fdupress May 26, 2025 12:32
@fdupress
Copy link
Member

@MM45, can you please give a quick description of the syntax and intended usage, either in a PR or in a wiki page? This will help me review, given this should probably be a usage-based review.

@MM45
Copy link
Contributor

MM45 commented May 29, 2025

Alright, so this most recent commit should fix all issues with external projects, but don't waste too much time on reviewing yet:

  • We want to still (1) prevent any non-documentation comments that are put before a documented item from being included in the source, and (2) not fail on a misplaced documentation comment (but only warn instead). For these two things, it would be nice if you could help me by taking a quick look @strub: It is taking me quite some time figuring out how to best approach this in the parser.
  • I will soon (TM) include an example "tutorial" file that explains all the functionality and what have you, which should both make reviewing this a lot easier, and it can immediately serve as (part of) the documentation.

@MM45
Copy link
Contributor

MM45 commented May 29, 2025

@MM45, can you please give a quick description of the syntax and intended usage, either in a PR or in a wiki page? This will help me review, given this should probably be a usage-based review.

Well, that's a coincidence, I was just typing out my previous comment when you replied ^^ Anyways, I guess I should've changed this to a "draft PR", but I forgot. Sorry. See my previous comment. Only after fixing the things I mentioned there it should be reviewed (in my opinion), and I will ping here when that is the case.

@MM45 MM45 marked this pull request as draft May 29, 2025 14:41
@MM45
Copy link
Contributor

MM45 commented May 30, 2025

Okay @fdupress, if the checks succeed, I think it should be reviewable: the resources/tests/EasyCrypt_DocGen_Tutorial.ec file should be all you need to get going. Note that the fixes of the issues I mentioned in my first comment here are not there yet, but these are sort-off QOL changes anyways. Let me know if anything is wrong or unclear.

@MM45 MM45 marked this pull request as ready for review June 2, 2025 14:24
@MM45
Copy link
Contributor

MM45 commented Jun 2, 2025

Alright, now also the two QOL issues I mentioned before are dealt with (although there remains the warning from the parser that the new COMMENT token is not used. Do we care about this @strub?). Anyways, @fdupress, now it should really be reviewable, and I marked the PR as such^^

Copy link
Member

@fdupress fdupress left a comment

Choose a reason for hiding this comment

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

Must absolutely fix the gendoc/docgen discrepancy. (Spotted once; stopped looking.)
Otherwise good to go if all comments are considered and reasonably dismissable.

@MM45
Copy link
Contributor

MM45 commented Jun 17, 2025

Okidokie, I accidentally discovered a bug that made it such that documentation was generated for clones as if they were regular theories. Should be fixed now.

@strub
Copy link
Member Author

strub commented Jun 25, 2025

@fdupress Is the PR ok for you?

@fdupress
Copy link
Member

Yes, this is fine: Matthias has addressed my comments.

@MM45
Copy link
Contributor

MM45 commented Jun 25, 2025

@strub I made an attempt at what we discussed: preventing docgen from checking any proofs. Not sure if this is the right approach though. Also, I added a check that prevents exiting if there is an eco file when doing docgen (I think this is desired behavior: probably you only generate docs after you check, in which case the eco file blocking docgen is annoying. However, we might consider a separate kind of eco for docgen at some point: it is not necessary to generate documentation anew when a file and its dependencies haven't changed)

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