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

Use full path for cross file links #444

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

renjiexu
Copy link

Hi team,

In our company, we're using namespace such as following:

company
├── account
│   └── rpc
│       └── v1
│           └── account_api.proto
├── webhook
│   └── v1
│       └── webhook.proto
|       └── api.proto

instead of generating one html for all the *.proto, we generate one for each namespace:

company
├── account
│   └── rpc
│       └── v1
│           └── index.html
├── webhook
│   └── v1
│       └── index.html

However, if webhook.proto refers a type in account_api.proto, the link would not work.
This PR would address the problem, confirmed it's working for me, would like your thoughts on this before adding specs.

(might be able to solve the request of #406)

Thanks,
Renjie

@alkasm
Copy link

alkasm commented Jul 30, 2021

Bumping this, would be really interested here as well. We have way too many protos to comfortably fit on one page, so the per-folder or per-package approach with cross-linking would be perfect.

@renjiexu
Copy link
Author

@alkasm good to know, I have a working solution for HTML/markdown in my forked version, will try to fix the unitest so we could have a clean PR to merge

@alkasm
Copy link

alkasm commented Jul 31, 2021

I think I found an issue with the generated links for nested message definitions. If you have

message Outer {
    message Inner { }
    Inner inner = 1;
}

Then protoc-gen-doc correctly makes the inner field link to #pkg.Outer.Inner, but this pr instead links to #/full/path/pkg.Outer.Inner. Either the initial # needs to be removed, or the full path needs to be removed when they're in the same file.

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.

None yet

2 participants