[doc] Add ldoc @supermodule tag to draw inheritance diagram and manage inherited properties #3288
Conversation
|
Nice work! I think we should eventually add a commit to remove the "useless" methods from Beside, I think we can merge this as-is and fix this in other PRs? I am trying to finish my old (over 1 years old...) PR to upgrade the landing page. With both of these merged (and the fix to have fewer methods), I am tempted to get 4.4 out of the door. |
|
Oops... Sorry @Elv13, I didn't realized you was asking for this PR to be ready ASAP because of the Travis issue and the migration to GitHub Actions can virtually block releases while we're working on it. Hence, we should do a release while we still have Travis credits. I don't think it would be nice to have this PR on the release without the additional transition work to complete what I started with 4fee67d. I also need to fix how I handled inheritance with This shouldn't be too much work, and I'm slowly working on it. But I don't think I'll be able to deliver a proper update before Sunday. So anyway, if we need to make a release now, I think we should skip this PR. If we can wait about 1 more week, I'll do my best to deliver! |
It isn't super urgent, I aim for May-ish. There is a lot of open bugs right now and some of them are totally worth fixing given how often we release stuff |
Aire-One
force-pushed
the
feature/ldoc-inheritance-diagram
branch
from
4fee67d
to
0cb0c23
Compare
|
I have reworked my code and pushed the usage of Looking my "
I added an ldoc tag Anyway, I think I reached a point where this PR gives a good/reliable render and is used enough in the documentation to make sense. |
Codecov Report
@@ Coverage Diff @@
## master #3288 +/- ##
=======================================
Coverage 88.66% 88.66%
=======================================
Files 701 702 +1
Lines 46643 46668 +25
=======================================
+ Hits 41354 41377 +23
- Misses 5289 5291 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Aire-One
force-pushed
the
feature/ldoc-inheritance-diagram
branch
from
0cb0c23
to
8f85667
Compare
Elv13
force-pushed
the
feature/ldoc-inheritance-diagram
branch
from
8f85667
to
3437e7b
Compare
|
I tried this and it works great! I made a commit with a few more I also added another commit to ask for your opinion. Feel free to delete it the next time you rebase this is you don't like it. What about laying the Hierarchy/Info/See section horizontally?
Individually, they are not very wide. So I think it makes the page a little less intimidating? I am not 100% happy with how it looks, so alternatives are welcome. Otherwise, everything seems great. If you want to merge this PR as it is and iterate to add more awesomeness, I am totally open to that. |
I quite like this one. Definitely less cluttered than the previous layout. One general issue I see is the fact that sometimes items are included as inherited when they aren't actually used. |
|
(@Elv13 I'm not sure why I had to rebase the whole branch to correctly merge your 2 commits. Did you apply modification on others commits? I can't see differences...)
I actually like it too!
I think we can fix that by overriding the property with a |
My implementation to hide a member in the documentation was incompatible with override members. I added a quick fixup commit to use another approach, more flexible (in regard to "override items to hide them"), but more ugly... Hiding the property with an override in |
|
Suggestion to wrap
The important change is diff --git a/docs/ldoc.css b/docs/ldoc.css
index a51fb80b..18544e54 100644
--- a/docs/ldoc.css
+++ b/docs/ldoc.css
@@ -522,8 +522,9 @@ pre .url { color: #272fc2; text-decoration: underline; }
.extra_header_box {
display: flex;
flex-direction: row;
+ flex-wrap: wrap;
}
.extra_header_section {
- flex: 1;
+ flex-grow: 1;
}I also replaced the nested diff --git a/docs/ldoc.css b/docs/ldoc.css
index a51fb80b..436dcd9b 100644
--- a/docs/ldoc.css
+++ b/docs/ldoc.css
@@ -505,25 +505,21 @@ pre .url { color: #272fc2; text-decoration: underline; }
}
/* Inheritance diagram */
-.inheritance .inheritance__level {
- list-style: none;
+.inheritance {
+ margin-bottom: 15px;
+ margin-left: 15px;
}
-.inheritance .inheritance__level__node::before {
- content: "↳";
-}
-
-.inheritance .inheritance__level__node--root::before {
- /* simulate the spacing of the arrow character */
- content: " ";
- white-space: pre;
+.inheritance .inheritance__level > .inheritance__level {
+ margin-left: 30px;
}
.extra_header_box {
display: flex;
flex-direction: row;
+ flex-wrap: wrap;
}
.extra_header_section {
- flex: 1;
+ flex-grow: 1;
}
diff --git a/docs/ldoc.ltp b/docs/ldoc.ltp
index 9ccfff00..1a0c0b7a 100644
--- a/docs/ldoc.ltp
+++ b/docs/ldoc.ltp
@@ -199,12 +199,9 @@
<h3>Class Hierarchy</h3>
<div class="inheritance">
# local function draw_hierary_recursifly(i)
- <ul class="inheritance__level">
-# if i == #hierarchy then
- <li class="inheritance__level__node inheritance__level__node inheritance__level__node--root">
-# else
- <li class="inheritance__level__node inheritance__level__node">
-# end
+ <div class="inheritance__level">
+ <span>
+ <span>↳</span>
# local mod = hierarchy[i]
# local name = display_name(hierarchy[i])
# if mod == module then
@@ -212,13 +209,11 @@
# else
<a href="$(ldoc.ref_to_module(mod))">$(name)</a>
# end
- </li>
+ </span>
# if i > 1 then
- <li>
-# draw_hierary_recursifly(i - 1)
- </li>
+# draw_hierary_recursifly(i - 1)
# end
- </ul>
+ </div>
# end -- module.tags.supermodule
# draw_hierary_recursifly(#hierarchy)
</div> |
This commit adds a new ldoc custom tag `@supermodule`. It has to be used at the module level. It should refer to the module supermodules. This tag can be used multiple time by the same module, but we ignore other calls (for now?) as (AFAIK) we only use one way inheritance. This tag is used in the ldoc template to find modules hierarchy and draw the inheritance tree. It makes it easy to find and navigate to parents modules.
This commit uses the `@supermodule` tag to recursively find all the properties from supermodules and add them to the current module documentation.
For some reasons, sometime `item.inherited` is `false` even if the item was added to the `all_module_kinds` table by the "hierarchy lookup" for-loop, and we already force the `inherited` property to be sets to `true` at this moment. With this commit, we add a second fail-check condition based on the `item.baseclass` property to determine if the item is inherited or not when we do the render.
Add a new ldoc tag `@hidden`. This tag allows us to keep documentation for magic methods (e.g. `wibox.widget.base:get_visible`) but prevent it from being part of the documentation used by final users.
For some even more strange reasons than commit 047729a, it seems we can also have the error that `item.inherited` can be sets to `true` on items that are not inherited... I just had this issue with `wibox.widget.imagebox` where properties were marked as inherited from `wibox.widget.imagebox`. Best way to fix it is to only trust the check on `item.baseclass`, and completly dismiss its `inherited` property.
Co-authored-by: Lucas Schwiderski <lucas@lschwiderski.de> Co-authored-by: Aire-One <Aire-One@users.noreply.github.com>
Add an explicite `@property` tag to the `wibox.layout.flex` doc comments to override the `fill_space` property from `wibox.layout.fixed` and mark it as hidden thanks to `@hidden`.
Aire-One
force-pushed
the
feature/ldoc-inheritance-diagram
branch
from
4c45021
to
b184f95
Compare
|
@sclu1034, I have kept your first diff about flex wrapping. I think nested lists are better to draw trees with a regard to data structure and accessibility, tho, so I dismissed the second part. @Elv13 I have changed in your extra-header feature the class name to better match BEM naming convention (http://getbem.com/naming/) and I used As a side note, I have also clean up the commits history and rebase on current master. If we don't have more changes request or places where |
|
Thanks a lot for this! |
|
The doc has been regenerated using this. Overall, it seems to work. Some pages like https://awesomewm.org/apidoc/core_components/screen.html will need some fixup, it seems some tag don't close. I also managed to introduce some broken link in the landing page https://awesomewm.org/apidoc/index.html, I will fix that next week |
|
this class hierarchy thing is really cool 👍 |
@Elv13 I think I closed one |
This is a new implementation of the "let's draw a hierarchy diagram" thing I started with #3263. For this implementation, I used, as @Elv13 suggested, a new ldoc tag.
This adds a new ldoc tag:
@supermodule. We can use it to indicate the module's supermodule. The logic to build the hierarchy tree and find supermodules contents is implemented in the template itself. I couldn't find a better solution and this is what was used in this other project (https://gricad-gitlab.univ-grenoble-alpes.fr/brenona/arcades/-/blob/master/assets/ldoc/ldoc.ltp#L24-56) I found while digging ldoc (lunarmodules/ldoc#293)...Anyway, the last commit (4fee67d) is actually a POC, so you can easily see what this PR is about (to compare with https://awesomewm.org/apidoc/widgets/wibox.widget.textclock.html):
https://imgur.com/a/EBgVpuO
https://user-images.githubusercontent.com/6602958/111206863-3d66cc80-85c9-11eb-83ac-995f006b1951.png
(links as it was really long and kinda spam the page)
This isn't perfect and still needs some adjustments. The goal would be to completely remove the use of
docs/common/object.ldocanddocs/common/widget.ldocas the content of these file should be automatically added by inheritance.Looking at the length of the final render, I think we probably shouldn't include all the descriptions and contents from supermodules. Maybe we could try another approach similar to what the Android API references does (for example: https://developer.android.com/reference/android/widget/ImageView, see the "Inherited *" sections). I'm open to feedbacks and suggestions 😄
Also, I'd like some inputs on how we should show the root classes (
gears.objectvswibox.widgetvswibox.widget.base).The
wibox.widget.base.make_widget()function (used as a base for widgets) returns an instance ofgears.objectmonkeypatched with the internalwibox.widget.base.widgetproperties and additional properties that we once calledwibox.widgetfordocs/common/widget.ldoc...