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

feat(tactic): make docstrings of imported modules accessible #81

Merged
merged 2 commits into from
Nov 22, 2019

Conversation

Vtec234
Copy link
Collaborator

@Vtec234 Vtec234 commented Nov 11, 2019

Changes the semantics of module_doc_strings to contain a list of (some filename, top-level-docstring) pairs, plus a (none, top-level-docstring) for the current module. If a single module contains multiple top-level docstrings, they're concatenated with newlines (but I could make it a list like before if people want). Declaration docstrings are no longer present in the output, since they're accessible via doc_string anyway.

CC @robertylewis

@robertylewis
Copy link
Member

Cool, thanks!

If a single module contains multiple top-level docstrings, they're concatenated with newlines (but I could make it a list like before if people want).

I think a list would be better. No reason to throw away information about how they're separated. Even better: would it be possible to pair each docstring with its location in the file?

@robertylewis
Copy link
Member

The motivation there is that when we're generating docs, it would be nice to have some kind of sectioning. Module docs in the middle of the file would be the easiest way to do this. But we need to know where to put them in the generated docs.

@Vtec234
Copy link
Collaborator Author

Vtec234 commented Nov 14, 2019

It's now a list of (source filename, [(position info, doc string)]).

@Vtec234
Copy link
Collaborator Author

Vtec234 commented Nov 14, 2019

Here is a possible log entry for this:

  • Changes the semantics of tactic.module_doc_strings to return a list of module-level documentation entries for each imported file, and not just the currently open one.

@Vtec234 Vtec234 force-pushed the doc_output branch 3 times, most recently from 19760af to 5f0bb4f Compare November 16, 2019 01:22
@Vtec234
Copy link
Collaborator Author

Vtec234 commented Nov 16, 2019

New version adds olean_doc_strings and reimplements module_doc_strings on top of that.

@Vtec234
Copy link
Collaborator Author

Vtec234 commented Nov 18, 2019

Hm, that CI failure is really quite mysterious. I have no clue why this PR would change the column number printed in an unrelated test, and only on MacOS of all things.

@bryangingechen
Copy link
Collaborator

bryangingechen commented Nov 18, 2019

It's yet another instance of #72. It should go away if the job is re-run. @cipher1024

@robertylewis
Copy link
Member

I haven't pushed it yet (still needs some cleanup), but I updated my doc generation tool to use this. Worked exactly as expected. Thanks again!

@cipher1024 cipher1024 merged commit e1fe877 into leanprover-community:master Nov 22, 2019
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.

4 participants