-
Notifications
You must be signed in to change notification settings - Fork 15
Ignore commented out declarations #224
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
base: master
Are you sure you want to change the base?
Conversation
| ## searched cause problems otherwise. | ||
| @DONT_SCAN_NEXT_LINE := function() | ||
| ReadLineWithLineCount( filestream ); | ||
| end, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being able to get rid of @DONT_SCAN_NEXT_LINE is a nice side benefit, IMHO.
2fed9e0 to
79f6397
Compare
Codecov Report
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
+ Coverage 74.95% 74.98% +0.03%
==========================================
Files 13 13
Lines 2336 2347 +11
==========================================
+ Hits 1751 1760 +9
- Misses 585 587 +2
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #224 +/- ##
==========================================
+ Coverage 74.95% 74.98% +0.03%
==========================================
Files 13 13
Lines 2336 2347 +11
==========================================
+ Hits 1751 1760 +9
- Misses 585 587 +2
|
|
We use the fact that commented operations are parsed to add documentation for operations which are already installed for more general filters, see e.g. https://github.com/homalg-project/FinSetsForCAP/blob/25c0b9b02752de6d468d19fd79d2d13e16dbb41d/gap/FinSetsForCAP.gd#L120 It would be nice to have a replacement for this. |
|
Since people are using this, this PR can't be merged as-is. I propose the following strategy:
|
|
I have two other ideas for an alternative mechanism (I'm not suggesting they are better than you're approach, just two random ideas):
|
Fixes #66 and generally makes AutoDoc parsing a bit stricter.
Now that I think about it: a few people may have abused the fact that GAP parses commented out declarations... Alas, we never documented this, and I'd argue that we never should have parsed those. People who abused it this way should file issues with us telling us about their use cases, so that we can support it properly.