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

Folder Structure Links #104

Open
bwood4 opened this issue Apr 8, 2022 · 9 comments
Open

Folder Structure Links #104

bwood4 opened this issue Apr 8, 2022 · 9 comments

Comments

@bwood4
Copy link

bwood4 commented Apr 8, 2022

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
Copy link
Owner

Doraku commented Apr 9, 2022

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
Copy link
Author

bwood4 commented Apr 11, 2022

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
Copy link
Owner

Doraku commented Apr 11, 2022

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
Copy link

aremes commented May 19, 2022

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
Copy link
Owner

Doraku commented Jun 20, 2022

see #107 (comment)

@IdkGoodName
Copy link
Contributor

Wouldn't it make more sense to have

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

rather than

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

?

@Doraku
Copy link
Owner

Doraku commented Jun 28, 2022

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
Copy link

aremes commented Jun 29, 2022

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
Copy link

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.

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

No branches or pull requests

5 participants