Skip to content

Add colons between parameter names and types in docs #489

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

Merged
merged 4 commits into from
Nov 5, 2019

Conversation

mrotondo
Copy link
Contributor

@mrotondo mrotondo commented Oct 3, 2019

This addresses the change in the doc generation parser which was swallowing the colon. Fixes #468

Copy link
Member

@Mr0grog Mr0grog left a comment

Choose a reason for hiding this comment

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

Well, I feel really bad about this, but it turns out this is not a good fix for the problem. That’s not your fault, though! The issue was incorrectly described in #468. When reviewing this, I noticed it didn’t quite fix the whole problem and wound up going on a deep dive to figure out what was really happening.

As a result, I added a deeper, correct analysis as a comment on the original issue: #468 (comment)

Not sure if you want to keep working on the issue and whether you want to keep working on this PR or just start over. Let me know.

@mrotondo
Copy link
Contributor Author

mrotondo commented Oct 5, 2019

Thanks for digging into this! I'll stick with the issue, the actual fix will be more satisfying than this commit, which definitely felt arbitrary!

…fore" rule that inserts ": " before API parameters' types
@mrotondo
Copy link
Contributor Author

This PR now adds the custom styling which properly causes API parameters to have ": " inserted before their type. In #468, you also mention adding "nicer styling for field lists", is there a particular aspect of the current (definitely very plain) list formatting that you think should be improved?

@Mr0grog
Copy link
Member

Mr0grog commented Nov 1, 2019

I don’t think I had any particularly strong ideas here. The biggest thing for me was perhaps to visually distinguish the headings (e.g. “Parameters,” “Returns,” etc.) in a clearer way from the actual list items, since they both have the same styling right now.

You could also try matching it up with the old styling (where those headings were to the left of the list of parameters/return values/etc). CSS grid would probably be the easiest way to do that (just spitballing here, not really well tested):

.rst-content .field-list.simple {
  display: grid;
  grid-template-columns: 10em 1fr;
}

.rst-content .field-list.simple > dt {
  background: transparent;
  border: none;
  display: block;
}

But you could also just leave it for now, and I or someone else with stronger layout ideas could come back around later to improve.

@mrotondo
Copy link
Contributor Author

mrotondo commented Nov 5, 2019

I'm gonna leave this alone for now, since it's more legible than it was and the class constructor field lists don't have quite the same css structure as the method field lists. This should be good to go

@Mr0grog
Copy link
Member

Mr0grog commented Nov 5, 2019

Great, merging this, then. 😄

@Mr0grog Mr0grog changed the title Removes spaces from between parameter names and colons in docstrings Add colons between parameter names and types in docs Nov 5, 2019
@Mr0grog Mr0grog merged commit 4dd5c40 into edgi-govdata-archiving:master Nov 5, 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.

Parameter Names and Types are Displayed Wrong in Docs
2 participants