search

RicoSuter / Namotion.Reflection

.NET library with advanced reflection APIs.
MIT License
213 stars 43 forks source link

How to set custom XML docs path #23

Open LockTar opened 4 years ago

LockTar commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

https://github.com/RicoSuter/NJsonSchema/pull/1087

LockTar commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

LockTar commented 4 years ago

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

RicoSuter commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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.