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

Issues with type bound procedures: incomplete source code and 404 #450

Closed
HugoMVale opened this issue Oct 2, 2022 · 11 comments · Fixed by #451
Closed

Issues with type bound procedures: incomplete source code and 404 #450

HugoMVale opened this issue Oct 2, 2022 · 11 comments · Fixed by #451

Comments

@HugoMVale
Copy link

HugoMVale commented Oct 2, 2022

Hello. First of all, let me thank you for this very helpful piece of software.
I noticed 2 issues related to derived data types and type-bound procedures:

  1. The source code box only shows the code of the type definition, but not the code for the associated procedures. See example here.
  2. The "Read more..." in type-bound procedures returns 404. See example here.

Thanks!

@ZedThree
Copy link
Member

ZedThree commented Oct 3, 2022

  1. This is actually the expected behaviour, the source code for the type-bound procedures will be on the procedure's individual page, rather than included with the type.
  2. This is a bit unfortunate -- those procedures are private and the default display argument is public, protected, meaning that we don't generate individual pages for those procedures. But because they are still accessible through the type, we should perhaps still generate those pages.

@cmacmackin
Copy link
Contributor

But because they are still accessible through the type, we should perhaps still generate those pages.

I think that would be ill-advised. If the implementation is private and you don't want to see private things then it should not have a dedicated page of documentation for it. Only the interface should be documented, in the type-binding. However, FORD should also be designed not to generate a broken link in that case (I'm surprised that wasn't already the case!).

@ZedThree
Copy link
Member

ZedThree commented Oct 3, 2022

True, but type-bound procedures are a bit weird like that -- if the type is public then so are its procedures (unless made explicitly private), but the un-bound names may still be private. I don't think they are implementation details in that case, it's just forbidding one name (call foo(bar)) in preference of another (call bar%foo()).

I'm sure how FORD should indicate that, but I think the simple fix of "public type-bound procedures are public procedures" is the most expedient, at least for the time being.

@cmacmackin
Copy link
Contributor

It's only the procedure bindings which will be public in that case; not the procedures themselves. The documentation being discussed here is for the bindings only, not for the actual procedures. FORD is designed to maintain that distinction. If we were to take the approach that the implementations of public type-bound procedures should automatically be treated as public then we would have to apply that to overloads in public generic interfaces as well (which are currently treated the same way as type-bound procedures).

I think that it is useful to maintain the current behaviour as it reinforces the fact that type-bindings of procedures are distinct from the actual procedures themselves. Unlike in C++, for example, these procedures have not automatic access to variables of the derived type. They exist totally separately from the derived type and should therefore have their own visibility logic.

If we were to go down the route you propose, I would at least suggest making it a configurable option.

@HugoMVale
Copy link
Author

Hello. Thanks for having a look at this.

  1. Ok, I can understand that such is the expected behavior.
  2. Here, honestly, it seems to me that the current approach used by FORD somehow defeats the purpose of documentation! It feels like all type-bound procedures become downgraded in terms of documentation possibilities: one gets 4 lines, period. Note also that this happens even if there is no procedure renaming (a=>b), as here for weno%reconstruct.

@ZedThree
Copy link
Member

ZedThree commented Oct 3, 2022

@cmacmackin Hmm, I take your point. I can just rejig the templates a little to display the full doc for each binding on the type page only.

I'll have to check to see if something similar happens to generic interfaces (and how generic type-bound procedures are displayed too).

It does raise some other questions though -- should we be reporting the procedure names for the bindings if these are implementation details? For example, init => mstvd_init in @HugoMVale's first post, should it really look like:
image

and only link to the specific procedure page if that is public?

@ZedThree
Copy link
Member

ZedThree commented Oct 4, 2022

@HugoMVale I've fixed the "Read More" bit not working in #451 -- if you're able, would you mind just checking that works ok for you?

I've got a couple of other small bits I want to get in, then I'll make a new release

@HugoMVale
Copy link
Author

@HugoMVale I've fixed the "Read More" bit not working in #451 -- if you're able, would you mind just checking that works ok for you?

I've got a couple of other small bits I want to get in, then I'll make a new release

Hello @ZedThree. Just tried it on my computer and the full documentation of each public type-bound procedure is now completely displayed. Thanks!
There is, however, something I do not fully understand:

  • If I select Derived Types > grid1, I can see the source code corresponding to the type definition at the bottom of the page. link.
  • In contrast, for other types like weno, mstvd etc., there is no equivalent source code box. link

There must be some sort of difference between grid1 and the other types, but I can't see it. What am I overlooking?

@ZedThree
Copy link
Member

ZedThree commented Oct 5, 2022

That's very odd. In both cases, the component docstrings aren't displayed either, even though they're all documented. I'll investigate

@ZedThree
Copy link
Member

ZedThree commented Oct 5, 2022

@HugoMVale Ok, I found it, FORD needs the name on the end statement to find the source to include on the page. This should spit out a warning, but it's gated behind the warn setting -- which I think is only supposed to be for warning about undocumented entities.

Because we work almost entirely off regular expressions, this is a bit tricky to do robustly, so the easiest fix is to add the type name at the end, so end type weno.

EDIT: The components not being visible is a separate issue that I'm still investigating

@HugoMVale
Copy link
Author

Many thanks, @ZedThree !

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 a pull request may close this issue.

3 participants