Doraku / DefaultDocumentation

Create a simple markdown documentation from the Visual Studio xml one.
MIT No Attribution
157 stars 26 forks source link

Folder Structure Links #104

Open bwood4 opened 2 years ago

bwood4 commented 2 years ago

Hey I'm messing around with your FolderFilenameFactory sample (From #94), and it looks like the links generated on docs within subfolders don't work because the links aren't relative paths.

So if I have a structure like this: OutputFolder\ ----TopLevel.md ----TopLevel\ --------SubLevel.md

The link in SubLevel.md to TopLevel.md is still just "TopLevel.md" rather than "../TopLevel.md".

Adding some code in the markdown generation to use relative paths would be a little more robust to different configurations, but if there's a way to solve this with a plugin I'd be happy with that too.

This tool is awesome BTW, thank you for your work!

Doraku commented 2 years ago

oh that's a big shortcoming of the sample :/ You should be able to fix this with a custom IUrlFactory to replace the existing one for DocItem.

    public sealed class DocItemAbsoluteFactory : IUrlFactory
    {
        // so it override the default one
        public string Name => "DocItem";

        public string GetUrl(IGeneralContext context, string id)
        {
            if (!context.Items.TryGetValue(id, out DocItem item))
            {
                return null;
            }

            if (item is ExternDocItem externItem)
            {
                return externItem.Url;
            }

            DocItem pagedItem = item;
            while (!pagedItem.HasOwnPage(context))
            {
                pagedItem = pagedItem.Parent;
            }

           // only difference with the default implementation is that you should append the links base url to all urls to make them absolute instead of relative
           // since the urls are only generated once and cached for later use, there's no way to make them aware of a relative page change
            string url = context.Settings.LinksBaseUrl + context.GetFileName(pagedItem);
            if (context.GetRemoveFileExtensionFromUrl() && Path.HasExtension(url))
            {
                url =url.Substring(0, url.Length - Path.GetExtension(url).Length);
            }
            if (item != pagedItem)
            {
                url += "#" + PathCleaner.Clean(item.FullName, context.GetInvalidCharReplacement());
            }

            return url;
        }
    }

I have not tested the code above but hopefully it should help you make it works

bwood4 commented 2 years ago

Took a couple things to get that method to work:

  1. PathCleaner is internal
  2. GetRemoveFileExtensionFromUrl() and GetInvalidCharReplacement() were both unavailable, maybe from a different assembly?

I copied the code for those over to my plugin and got it working.

I guess with this method you'd want to put the Url of your repo in LinksBaseUrl? This didn't seem to work in my use case (I'm putting it into an AzureDevops wiki). The wiki actually generates a unique id for each page URL and doesn't use the path, so linking to an absolute URL doesn't play nicely.

One possibility of getting relative paths if you are interested could be to change the AppendLink() extension methods of IWriter, and add extension methods to IContext that accept a secondary DocItem or id. The urls generated are already in a psuedo-absolute format, so when appending the links, the cached url of the desired link and the cached url of the IWriters current DocItem could be used to determine the actual relative path using something like Path.GetRelativePath() (available with .Net Standard 2.1, but there are other implementation floating around).

Something like this:

public static class IGeneralContextExtension
{
    public static string GetUrl(this IGeneralContext context, DocItem relativeToItem, DocItem item)
        => Path.GetRelativePath(Path.GetDirectoryName(context.GetUrl(relativeToItem)), context.GetUrl(item));
}

And then use that in the AppendLink() methods of the writer:

public static class IWriterExtensions
{
    public static IWriter AppendLink(this IWriter writer, DocItem item, string displayedName = null) 
        => writer.AppendUrl(writer.Context.GetUrl(writer.DocItem, item), displayedName ?? item.Name, item.FullName);
}

You wouldn't have to change any API, although adding a setting to turn this off and on might be good in case people want the absolute urls.

EDIT:

I actually did get your method to work for Azure Devops wiki by using "/" for LinksBaseUrl! You have to use the folder that you publish as the wiki for the LinksBaseUrl, so if your folder structure is docs/API/ and you generate documentation to the "API" folder and publish the "docs" folder as wiki, LinksBaseUrl should be "/API/". It's still not as robust as relative paths because it doesn't work outside of the wiki and relies on which folder you publish, but it's good enough. Feel free to close this issue or leave it open depending on if you want to pursue relative paths, I do think there is still some value in it.

Thanks for your help!

Doraku commented 2 years ago

Thanks for your detailed walkthrough! First sorry for completely missing that I was using some internal code in the default implementation, I intend to make DefaultDocumentation.Markdown available as a package too in the futur to help people create features on top of it instead of having to recreate/copy past everything but it still needs some cleaning.

Great that you found a workaround for your wiki and I totally get the limitation of absolute paths in this case.

Calculating the relative path using IWriter.DocItem would work well, my only reserve is that as you said it is something that need to be handled directly in the ISection implementations, it can't be injected in existing ones. It goes against my goal of being lazy by letting people create what they need as plugins so maybe the api is still not open enough to allow those kind of feature, or this is a case that is required to be handled directly by my code... I'll keep this issue open as a reminder to do some more thinking on this case :)

aremes commented 2 years ago

I'd very much appreciate relative paths in this case too. I have a similar setup using a modified version of the FolderFilenameFactory sample. Absolute URLs are not an option in my case.

Doraku commented 2 years ago

see https://github.com/Doraku/DefaultDocumentation/issues/107#issuecomment-1160889885

IdkGoodName commented 2 years ago

Wouldn't it make more sense to have

Out
|- SomeType
    |- index.md
    |- SomeProperty.md

rather than

Out
|- SomeType.md
|- SomeType
    |- SomeProperty.md

?

Doraku commented 2 years ago

I mean that's up to you I just made a example of how to implement it (in the simplest way) in the sample :)

aremes commented 2 years ago

Hi @IdkGoodName !

This is what i needed too, so i thought i'd give you a hint to get you started.. basically, in your FileNameFactory implementation, you'd have to check for the type of documentitem and then set the filename to "index.md" manually. The thing is, this depends on the structure you're generating (<DefaultDocumentationGeneratedPages> setting). In my case, i'm not generating separate pages for properties so i'm only using index.md on AssemblyDocItem and NamespaceDocItem. In your case you'd probably have to do it for TypeDocItem as well.

Here's the relevant part of my own FolderFileNameFactory

public string GetFileName(IGeneralContext context, DocItem item)
{
    if(item is null)
        throw new ArgumentNullException(nameof(item));
    if(item is NamespaceDocItem || item is AssemblyDocItem)
    {
        return Path.Combine(item.FullName.Split('.').Concat(Enumerable.Repeat("index.md", 1)).ToArray());
    }
    else
    {
        //proceed as usual
    }
}
juliansangillo commented 1 year ago

I am trying to implement my own FolderFileNameFactory in my plugin and am running into this issue as well. It would be great if we could find a solution to this.