Generate enum constant Documentation #74
Conversation
Signed-off-by: nikkirad <nikkizrad@gmail.com>
Signed-off-by: nikkirad <nikkizrad@gmail.com>
Signed-off-by: nikkirad <nikkizrad@gmail.com>
Signed-off-by: nikkirad <nikkizrad@gmail.com>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #74 +/- ##
==========================================
- Coverage 42.40% 42.25% -0.16%
==========================================
Files 2 2
Lines 540 542 +2
==========================================
Hits 229 229
- Misses 311 313 +2
☔ View full report in Codecov by Sentry. |
Signed-off-by: nikkirad <nikkizrad@gmail.com>
Signed-off-by: nikkirad <nikkizrad@gmail.com>
nikkirad
changed the title
| @@ -617,7 +620,7 @@ def get_index_text(self, modname, name_cls): | |||
| return _('%s() (built-in %s)') % \ | |||
| (name_cls[0], self.chpl_type_name) | |||
| return _('%s() (in module %s)') % (name_cls[0], modname) | |||
| elif self.objtype in ('data', 'type', 'enum'): | |||
| elif self.objtype in ('data', 'type'): | |||
There was a problem hiding this comment.
I think this is here because you can declare enums inside a class as well as outside (see test/classes/bradc/enumInClass.chpl, which appears to work today). So I think it would be good to keep this, or at least write a test case that does so and ensure it works the same before and after your change (and maybe ensure you can still follow links to it).
There was a problem hiding this comment.
Before the change, it would print in module <module name> even if it was declared inside of something else (i.e. record). Now it prints enum in <wherever the enum is in>. Here's an example of the change:
Before:
After:
Just added a test case that presents this change.
There was a problem hiding this comment.
Well that definitely seems like an improvement
| @@ -416,6 +417,7 @@ def test_get_attr_like_prefix(self): | |||
| ('config const n', 'config const '), | |||
| ('blah blah blah blah blah', 'blah blah blah blah '), | |||
| ('enum Color', 'enum '), | |||
| ('enum constant USA', 'enum constant '), | |||
There was a problem hiding this comment.
Hmm. Not sure how I feel about this, since enum constant isn't Chapel syntax. It is descriptive, though . . .
There was a problem hiding this comment.
My understanding is that all of this is the stuff that gets output to the right of any .. <whatever>::, e.g. .. enumconstant:: enum constant USA, is that correct? If so, I'd be inclined to not output that second part and just rely on the test on line 414, (so we'd generate .. enumconstant:: USA instead, I think?)
Or is this what is used to create those .. enumconstant:: USA pieces in the first place? In which case, it's fine to leave it as is.
(Sorry for not knowing the answer myself)
There was a problem hiding this comment.
Your understanding is correct, "enum constant" isn't necessary for the ..enumconstant:: to be outputted. My line of thinking for outputting enumconstant:: enum constant USA is only because of the cohesiveness with the rest of the documentation, simply a design decision that can be changed :)
There was a problem hiding this comment.
Then let's bring that up to the rest of the team, or at least Daniel and Ahmad. I don't want to accidentally steer us in a direction the rest of the team doesn't like just because I'm loud :)
| ('enum constant Pink', 'enum constant ', None, 'Pink', None), | ||
| ('enum constant December', 'enum constant ', None, 'December', None), | ||
| ('enum constant Hibiscus', 'enum constant ', None, 'Hibiscus', None), | ||
| ('enum constant Aquarius', 'enum constant ', None, 'Aquarius', None) |
There was a problem hiding this comment.
The answer to my question on line 420 may impact this as well?
Signed-off-by: nikkirad <nikkizrad@gmail.com>
|
After discussion with the team, the plan is to continue printing "enum constant," and reevaluate in the future when there are more rendered docs examples. |
This PR, along with [sphinxcontrib-chapeldomain PR #74](chapel-lang/sphinxcontrib-chapeldomain#74), contains changes made to `chpldoc.cpp` to generate documentation for each enum constant rather than only for the enum. Solution to #5022. Previously, documentation for enum constants have been either nonexistent, or written in different ways (such as a table) in order to be shown. Since this form of documentation is already available for other types that contain inner content (i.e. records), it is intuitive for enum constant documentation to exist as well. These changes allow chpldoc to recognize enum constants (written as `EnumElement` in the AST) as enum's children. However, only prefix comments will be recognized and documented. There is intent to allow suffix comments to be recognized and documented in the future. third-party/chpl-venv/Makefile and make_doc errors should be resolved after merging [this PR](chapel-lang/sphinxcontrib-chapeldomain#74)
Implement
enumconstantand its documentation. These changes, along with PR #22750, allowchpldocto generate documentation for each enum constant rather than only for the enum. Solution to chapel-lang/chapel#5022.Previously, documentation for enum constants have been either nonexistent, or written in different ways (such as a table) in order to be shown. Since this form of documentation is already available for other attribute-like types (i.e. records), it is intuitive for enum constant documentation to exist as well.
This PR changes
enumto a Class Object andenumconstantto a Class Member (suggested by Thomas Van Doren in 2015). It includes test cases forenumconstantand presents the changes made in PR #22750.An example of affected code:
