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

How to set custom XML docs path #23

Open
LockTar opened this issue Oct 31, 2019 · 13 comments
Open

How to set custom XML docs path #23

LockTar opened this issue Oct 31, 2019 · 13 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@LockTar
Copy link

LockTar commented Oct 31, 2019

Hi,

I'm trying to help with the following issue that generates documentation via NSwag in Azure Functions v2. I know what the problem is but I need to determine where NSwag get's the xml documentation from.

I think it's coming from this package.

In order to solve the issue, I need to be able to set a custom xml file path. In Azure Functions on the server, this is different then the path when you locally develop.
So I tracked the code through NSwag to this package and ended up with the method GetXmlDocsPath.

So the ultimate questions are:

  1. Is this the correct line that NSwag is using to obtain the XML file for the documentation?
  2. If so, How can I set a custom path to the XML file in NSwag and then of course here in this package?
@RicoSuter
Copy link
Owner

Yes, the path is "calculated" in GetXmlDocsPath() which is static and probably cannot be configured at the moment.

https://github.com/RicoSuter/Namotion.Reflection/blob/master/src/Namotion.Reflection/XmlDocsExtensions.cs#L616

The problem here is that this all is static and not an "injected" service, so globally changing the behavior is not clean because it might change the behavior of other xml docs consumers in the process.

@RicoSuter
Copy link
Owner

I've created a PR to move to an service instance so that you can actually modify the behavior - this is completely outdated as the code has been moved to this repo/library.

@LockTar
Copy link
Author

LockTar commented Oct 31, 2019

I've created a PR to move to an service instance so that you can actually modify the behavior - this is completely outdated as the code has been moved to this repo/library.

Which PR? In the NSwag repo?

@RicoSuter
Copy link
Owner

RicoSuter/NJsonSchema#1087

@LockTar
Copy link
Author

LockTar commented Oct 31, 2019

I've created a PR to move to an service instance so that you can actually modify the behavior - this is completely outdated as the code has been moved to this repo/library.

If I understand you sentence correctly, then Namotion.Reflection is outdated and you moved everything to NJsonSchema?

@RicoSuter
Copy link
Owner

RicoSuter commented Oct 31, 2019

No, the other way around: The XML docs functionality was in NJsonSchema directly but then moved to Namotion.Reflection so that it can be used in other contexts too... so maybe instead of calling namotion.reflection static xml docs directly, we should wrap them in a IXmlDocsService instance in NJS so that this service can be customized with eg a path or other xml docs sources...

@LockTar
Copy link
Author

LockTar commented Oct 31, 2019

Ok, I checked your PR. It is work in progress. I saw NJsonSchema XmlDocumentService was used in JsonSchemaGeneratorSettings. So if we indeed could give a list of file paths to the xml files, that would solve the problem.
Let me know if I can do something for you like testing.

I can create a fork of the NSwag.AzureFunctionsV2 and create a fix when your PR is implemented.

@RicoSuter
Copy link
Owner

The PR is super outdated and we cannot use it (lots of conflicts etc) it's just there as a sample.

@LockTar
Copy link
Author

LockTar commented Oct 31, 2019

I see. Let me take a look. Maybe I can help with this. No promises

@RicoSuter RicoSuter added enhancement New feature or request help wanted Extra attention is needed labels Jan 6, 2020
@RicoSuter
Copy link
Owner

RicoSuter commented Feb 19, 2020

I think the simplest way would be to add a global static func which is called to load the xml as fallback. This way ppl can hook into it and it’s quite easy to add... what do you think?

@LockTar
Copy link
Author

LockTar commented Feb 21, 2020

Well, to be honest I haven't looked at this issue since end 2019. I didn't had the time for it to dig into the code because it was more complex than I thought for a newbie on the project.
So I found another project that had almost the same functionality (only with swashbuckle).

Of course that wasn't working entirely but it was getting close. It has support for xml documentation but not multiple files from different class libraries. So I created multiple pull request there to add all the missing stuff. Maybe we can take a look from there?

See here the PR for multiple XML files.

I really need to understand your repo more to give a good answer. I also suspect that our discussion above is outdated and your further with removing NJsonSchema/moving to Namotion.Reflection?

@RicoSuter
Copy link
Owner

I also suspect that our discussion above is outdated and your further with removing NJsonSchema/moving to Namotion.Reflection?

Yes, I’d go with a Namotion.Reflection only solution (global resolve function as described). But if you dont need it anymore -and since noone else is requesting it - i’ll park this issue.

@LockTar
Copy link
Author

LockTar commented Feb 22, 2020

But if you dont need it anymore -and since noone else is requesting it - i’ll park this issue.

If noone else is requesting it, than that is for now the best thing to do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants